mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 10:13:11 +02:00
fix(github): recover existing apps and keep member linking separate
Explicit manual recovery switches to the existing-App identity path only after vault storage succeeds. Preserve the incomplete registration for local webhook repair. Account-only linking avoids manager setup APIs and retains personal GitHub connection onboarding. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
e3d8a0a47b
commit
01206ab4a5
4 files changed
+139
-24
No files matched your search
@@ -2108,6 +2108,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
webhookSecret,
|
||||
providerFetch,
|
||||
webhookSyncRequests,
|
||||
privateKey,
|
||||
setSupplementalProviderFetch(value: typeof supplementalProviderFetch) {
|
||||
supplementalProviderFetch = value;
|
||||
},
|
||||
@@ -2370,7 +2371,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
.where(eq(chatGitHubRegistrations.endpointId, bot.id));
|
||||
expect(session.status).toBe("failed");
|
||||
});
|
||||
it("wizard retains recovery when a manifest vault write saved App credentials but lost OAuth credentials", async () => {
|
||||
it("wizard retains partial manifest recovery until explicit existing-App recovery succeeds", async () => {
|
||||
const f = await reviewBotFixture();
|
||||
const returnState = randomUUID();
|
||||
const [connection] = await db
|
||||
@@ -2420,7 +2421,10 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
fetch: f.providerFetch,
|
||||
connector: () =>
|
||||
({
|
||||
githubApp: async () => ({ id: "partial-oauth" }),
|
||||
githubApp: async () => ({
|
||||
id: "partial-oauth",
|
||||
webhookUrl: "https://gateway.example.test/github-apps/partial-oauth/webhook",
|
||||
}),
|
||||
claimGitHubApp,
|
||||
}) as unknown as PaperclipCloudConnector,
|
||||
startDirect: f.service.startGitHubRegistration,
|
||||
@@ -2442,6 +2446,29 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
.from(chatGitHubRegistrations)
|
||||
.where(eq(chatGitHubRegistrations.endpointId, f.endpoint.id));
|
||||
expect(session.status).toBe("failed");
|
||||
const resourcesBefore = await f.service.listResources(f.endpoint.id);
|
||||
await f.service.storeGitHubApp(f.endpoint.id, "owner-user", {
|
||||
appId: f.endpoint.botExternalId!,
|
||||
privateKey: f.privateKey,
|
||||
webhookSecret: f.webhookSecret,
|
||||
});
|
||||
const [recovered] = await db.select().from(chatGitHubRegistrations)
|
||||
.where(eq(chatGitHubRegistrations.id, session.id));
|
||||
expect(recovered.handoff).toMatchObject({ cloudId: "partial-oauth", returnState });
|
||||
expect(recovered.handoff?.manifestClaimId).toBeUndefined();
|
||||
expect(recovered.status).toBe("failed");
|
||||
expect(await wizard.advance(f.endpoint.id, "owner-user")).toMatchObject({ state: "verify" });
|
||||
expect(claimGitHubApp).not.toHaveBeenCalled();
|
||||
expect(f.webhookSyncRequests).toEqual([
|
||||
expect.objectContaining({
|
||||
url: "https://gateway.example.test/github-apps/partial-oauth/webhook",
|
||||
secret: f.webhookSecret,
|
||||
}),
|
||||
]);
|
||||
expect((await f.service.listResources(f.endpoint.id)).map(({ id, enabled }) => ({ id, enabled })))
|
||||
.toEqual(resourcesBefore.map(({ id, enabled }) => ({ id, enabled })));
|
||||
expect((await f.service.get(f.endpoint.id)).botExternalId).toBe(f.endpoint.botExternalId);
|
||||
expect(finish).not.toHaveBeenCalled();
|
||||
});
|
||||
it("wizard recovers a renamed enrolled origin only after checking bound state and Cloud authority", async () => {
|
||||
const f = await seedCompany();
|
||||
|
||||
@@ -16,7 +16,7 @@ import { githubChatManagementService } from "./chat-github-management.js";
|
||||
import { githubReviewCheckService } from "./chat-github-checks.js";
|
||||
import { githubAutomaticReviewEvent, githubAutomaticAdmission, githubPreviousAssessment } from "./chat-github-events.js";
|
||||
import { githubReviewPrompt } from "./chat-github-review-policy.js";
|
||||
import { chatGitHubConfigurations, chatGitHubReviews } from "@paperclipai/db";
|
||||
import { chatGitHubConfigurations, chatGitHubRegistrations, chatGitHubReviews } from "@paperclipai/db";
|
||||
import type { GitHubReviewEventContext, GitHubReviewPolicy } from "@paperclipai/shared";
|
||||
import { githubChatReviewService } from "./chat-github-reviews.js";
|
||||
import { githubChatWizardService } from "./chat-github-wizard.js";
|
||||
@@ -38213,7 +38213,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
: null;
|
||||
}
|
||||
|
||||
async function storeGitHubApp(endpointId: string, userId: string, credentials: Record<string, string>) {
|
||||
async function storeGitHubApp(endpointId: string, userId: string, credentials: Record<string, string>, source: "manifest" | "existing_app" = "existing_app") {
|
||||
const initial = await endpointRecord(endpointId);
|
||||
if (!initial || initial.endpoint.provider !== "github") throw notFound("GitHub bot not found");
|
||||
await withCredentialMutationLease(initial.endpoint, async credentialLease => {
|
||||
@@ -38257,6 +38257,20 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
setup: { ...current.setup, github: { ...current.setup.github, stage: "install", appSlug: slug, installationUrl: `https://github.com/apps/${slug}/installations/new`, registrationStatus: "completed" } },
|
||||
healthMessage: "Install the App on GitHub to continue", updatedAt: new Date(),
|
||||
}).where(eq(chatEndpoints.id, endpointId));
|
||||
if (source === "existing_app") {
|
||||
// Explicit recovery uses the existing-App identity path. Only drop the
|
||||
// manifest requirement after the same App's credentials are vaulted;
|
||||
// leave registration incomplete so advance repairs and verifies delivery.
|
||||
await tx.update(chatGitHubRegistrations).set({
|
||||
handoff: sql`${chatGitHubRegistrations.handoff} - 'manifestClaimId'`,
|
||||
}).where(and(
|
||||
eq(chatGitHubRegistrations.companyId, record.endpoint.companyId),
|
||||
eq(chatGitHubRegistrations.endpointId, endpointId),
|
||||
eq(chatGitHubRegistrations.userId, userId),
|
||||
inArray(chatGitHubRegistrations.status, ["pending", "exchanging", "failed"]),
|
||||
sql`${chatGitHubRegistrations.handoff} ? 'manifestClaimId'`,
|
||||
));
|
||||
}
|
||||
await logActivity(tx as unknown as Db, { companyId: record.endpoint.companyId, actorType: "user", actorId: userId, action: "chat_github.app_connected", entityType: "tool_connection", entityId: record.endpoint.connectionId, details: { endpointId, appId: identity.botExternalId } });
|
||||
});
|
||||
});
|
||||
@@ -38278,7 +38292,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
|
||||
const githubRegistration = githubChatRegistrationService(db, {
|
||||
publicOrigin: () => getPublicBaseUrl(), webhookOrigin: () => getWebhookPublicBaseUrl(), fetch: fetchImpl,
|
||||
storeApp: async (endpointId, userId, app) => { await storeGitHubApp(endpointId, userId, { appId: app.appId, privateKey: app.privateKey, webhookSecret: app.webhookSecret, ...(app.clientId ? { clientId: app.clientId } : {}), ...(app.clientSecret ? { clientSecret: app.clientSecret } : {}) }); },
|
||||
storeApp: async (endpointId, userId, app) => { await storeGitHubApp(endpointId, userId, { appId: app.appId, privateKey: app.privateKey, webhookSecret: app.webhookSecret, ...(app.clientId ? { clientId: app.clientId } : {}), ...(app.clientSecret ? { clientSecret: app.clientSecret } : {}) }, "manifest"); },
|
||||
});
|
||||
|
||||
async function refreshGitHubRepositories(endpointId: string, userId: string) {
|
||||
@@ -38299,7 +38313,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
|
||||
const githubWizard = githubChatWizardService(db, {
|
||||
origin: () => getPublicBaseUrl() ?? options.githubWizardOrigin ?? null, fetch: fetchImpl, startDirect: githubRegistration.start, completeDirect: githubRegistration.complete, resumeDirect: githubRegistration.resumeRegistration,
|
||||
storeApp: storeGitHubApp,
|
||||
storeApp: (id, userId, credentials) => storeGitHubApp(id, userId, credentials, "manifest"),
|
||||
storeCredentials: async (id, userId, additional) => {
|
||||
const record = await endpointRecord(id);
|
||||
if (!record || record.endpoint.provider !== "github") throw notFound("GitHub bot not found");
|
||||
|
||||
@@ -411,6 +411,63 @@ describe("GitHub App wizard", () => {
|
||||
expect(githubChatApi.confirmIdentity).toHaveBeenCalledWith("draft-1", "42");
|
||||
expect(githubChatApi.startIdentity).not.toHaveBeenCalled();
|
||||
});
|
||||
it("lets a member add a personal connection without manager setup APIs", async () => {
|
||||
vi.mocked(githubChatApi.advance).mockRejectedValue(
|
||||
new Error("Manager permission required"),
|
||||
);
|
||||
await render("resume=draft-1&stage=identity");
|
||||
expect(
|
||||
container.querySelector('a[href="/apps/connect?source=github"]'),
|
||||
).toBeTruthy();
|
||||
expect(container.textContent).not.toContain("Manager permission required");
|
||||
expect(githubChatApi.advance).not.toHaveBeenCalled();
|
||||
expect(githubChatApi.startIdentity).not.toHaveBeenCalled();
|
||||
await click("Save & exit");
|
||||
expect(githubChatApi.saveDraft).not.toHaveBeenCalled();
|
||||
});
|
||||
it("links a member's verified personal account without advancing bot setup", async () => {
|
||||
vi.mocked(githubChatApi.personalConnections).mockResolvedValue([
|
||||
{
|
||||
connectionId: "personal-1",
|
||||
name: "My GitHub",
|
||||
login: "octocat",
|
||||
status: "active",
|
||||
enabled: true,
|
||||
},
|
||||
]);
|
||||
vi.mocked(githubChatApi.identity).mockResolvedValue({
|
||||
connectionId: "personal-1",
|
||||
githubUserId: "42",
|
||||
login: "octocat",
|
||||
avatarUrl: null,
|
||||
});
|
||||
await render("resume=draft-1&stage=identity");
|
||||
const account = container.querySelector(
|
||||
"#github-personal-account",
|
||||
) as HTMLSelectElement;
|
||||
await act(async () => {
|
||||
account.value = "personal-1";
|
||||
account.dispatchEvent(new Event("change", { bubbles: true }));
|
||||
});
|
||||
await click("Check my account");
|
||||
expect(githubChatApi.identity).toHaveBeenLastCalledWith(
|
||||
"draft-1",
|
||||
"personal-1",
|
||||
undefined,
|
||||
);
|
||||
await click("Confirm my account");
|
||||
expect(githubChatApi.identity).toHaveBeenLastCalledWith(
|
||||
"draft-1",
|
||||
"personal-1",
|
||||
"42",
|
||||
);
|
||||
expect(container.textContent).toContain(
|
||||
"You can now mention this bot on GitHub",
|
||||
);
|
||||
expect(githubChatApi.advance).not.toHaveBeenCalled();
|
||||
expect(githubChatApi.startIdentity).not.toHaveBeenCalled();
|
||||
expect(githubChatApi.confirmIdentity).not.toHaveBeenCalled();
|
||||
});
|
||||
it("completes automatically while reporting runtime readiness separately", async () => {
|
||||
fixture.endpoint = {
|
||||
...fixture.endpoint,
|
||||
|
||||
@@ -71,13 +71,14 @@ export function GitHubChatSetup() {
|
||||
});
|
||||
const [copied, setCopied] = useState(false);
|
||||
const [legacyAccount, setLegacyAccount] = useState("");
|
||||
const [identityLinked, setIdentityLinked] = useState(false);
|
||||
const [legacyIdentity, setLegacyIdentity] = useState<Awaited<
|
||||
ReturnType<typeof githubChatApi.identity>
|
||||
> | null>(null);
|
||||
const agents = useQuery({
|
||||
queryKey: ["github-setup-agents", selectedCompanyId],
|
||||
queryFn: () => agentsApi.list(selectedCompanyId!),
|
||||
enabled: !!selectedCompanyId,
|
||||
enabled: !!selectedCompanyId && !identityOnly,
|
||||
});
|
||||
const current = useQuery({
|
||||
queryKey: ["github-setup", resume],
|
||||
@@ -89,7 +90,7 @@ export function GitHubChatSetup() {
|
||||
const progress = useQuery({
|
||||
queryKey: ["github-wizard", resume],
|
||||
queryFn: () => githubChatApi.advance(resume!),
|
||||
enabled: !!bot && !existing,
|
||||
enabled: !!bot && !existing && !identityOnly,
|
||||
refetchInterval: (query) =>
|
||||
["connected", "create", "identity", "enrollment", "recovery"].includes(
|
||||
query.state.data?.state ?? "",
|
||||
@@ -106,7 +107,9 @@ export function GitHubChatSetup() {
|
||||
const selectedAgent = agents.data?.find(
|
||||
(agent) => agent.id === (bot?.assignedAgentId ?? agentId),
|
||||
);
|
||||
const state = progress.data;
|
||||
const state = identityOnly ? undefined : progress.data;
|
||||
const existingIdentityMethod =
|
||||
identityOnly || state?.identityMethod === "existing_connection";
|
||||
const connected = state?.state === "connected" && !existing;
|
||||
useEffect(() => {
|
||||
setBreadcrumbs([
|
||||
@@ -146,7 +149,7 @@ export function GitHubChatSetup() {
|
||||
}
|
||||
async function refresh() {
|
||||
await current.refetch();
|
||||
await progress.refetch();
|
||||
if (!identityOnly) await progress.refetch();
|
||||
}
|
||||
const appInput = () => ({
|
||||
name: (
|
||||
@@ -161,6 +164,7 @@ export function GitHubChatSetup() {
|
||||
const exit = () =>
|
||||
void run(async () => {
|
||||
if (
|
||||
!identityOnly &&
|
||||
bot &&
|
||||
!bot.botExternalId &&
|
||||
!state?.registration &&
|
||||
@@ -225,9 +229,12 @@ export function GitHubChatSetup() {
|
||||
? "Connect your account"
|
||||
: "Connect GitHub"}
|
||||
</h1>
|
||||
{(error || progress.error) && (
|
||||
{(error || (!identityOnly && progress.error) || accounts.error) && (
|
||||
<p role="alert" className="text-sm text-destructive">
|
||||
{error ||
|
||||
(accounts.error instanceof Error
|
||||
? accounts.error.message
|
||||
: undefined) ||
|
||||
(progress.error instanceof Error
|
||||
? progress.error.message
|
||||
: "Could not check GitHub setup. Try again.")}
|
||||
@@ -405,7 +412,15 @@ export function GitHubChatSetup() {
|
||||
</>
|
||||
) : state?.state === "identity" || identityOnly ? (
|
||||
<>
|
||||
{state?.identity ? (
|
||||
{identityOnly && identityLinked ? (
|
||||
<>
|
||||
<p role="status" className="text-sm">
|
||||
GitHub account <strong>{legacyIdentity?.login}</strong>{" "}
|
||||
connected. You can now mention this bot on GitHub.
|
||||
</p>
|
||||
{footer()}
|
||||
</>
|
||||
) : state?.identity ? (
|
||||
<>
|
||||
<p className="text-sm">
|
||||
Connect GitHub account <strong>{state.identity.login}</strong>{" "}
|
||||
@@ -437,6 +452,7 @@ export function GitHubChatSetup() {
|
||||
onChange={(event) => {
|
||||
setLegacyAccount(event.target.value);
|
||||
setLegacyIdentity(null);
|
||||
setIdentityLinked(false);
|
||||
}}
|
||||
>
|
||||
<option value="">Choose an account</option>
|
||||
@@ -461,15 +477,14 @@ export function GitHubChatSetup() {
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
{state?.identityMethod === "existing_connection" &&
|
||||
!legacyAccount && (
|
||||
<Link
|
||||
className="text-sm underline"
|
||||
to="/apps/connect?source=github"
|
||||
>
|
||||
Add a personal GitHub connection, then return to this draft
|
||||
</Link>
|
||||
)}
|
||||
{existingIdentityMethod && !legacyAccount && (
|
||||
<Link
|
||||
className="text-sm underline"
|
||||
to="/apps/connect?source=github"
|
||||
>
|
||||
Add a personal GitHub connection, then return to this draft
|
||||
</Link>
|
||||
)}
|
||||
{footer(
|
||||
legacyAccount
|
||||
? legacyIdentity
|
||||
@@ -484,14 +499,16 @@ export function GitHubChatSetup() {
|
||||
legacyIdentity?.githubUserId,
|
||||
);
|
||||
setLegacyIdentity(observed);
|
||||
if (legacyIdentity) await refresh();
|
||||
if (legacyIdentity) {
|
||||
setIdentityLinked(true);
|
||||
await refresh();
|
||||
}
|
||||
} else {
|
||||
const result = await githubChatApi.startIdentity(bot.id);
|
||||
window.location.assign(result.authorizationUrl);
|
||||
}
|
||||
},
|
||||
state?.identityMethod === "existing_connection" &&
|
||||
!legacyAccount,
|
||||
existingIdentityMethod && !legacyAccount,
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
|
||||
Reference in new issue
Block a user