mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 20:46:28 +01:00
fix(github): preserve legacy repository selection and qualify fixtures
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
87945c8cf3
commit
f46360a351
5 files changed
+75
-52
No files matched your search
@@ -106,7 +106,7 @@ enrollment handoff passed. The test did not authorize Cloud enrollment or
|
||||
create a provider App.
|
||||
|
||||
The focused wizard and retained clipboard/recovery UI suites pass (37 tests).
|
||||
Backend wizard coverage passes (14 cases), including both owner types,
|
||||
GitHub backend coverage passes (189 cases), including both owner types,
|
||||
interrupted registration and vault storage, empty saved repository selections,
|
||||
localhost configuration, concurrent identity attempts, enrolled-origin recovery,
|
||||
tenant binding, and legacy callbacks. The complete OpenAPI suite passes (13
|
||||
@@ -115,16 +115,19 @@ browser journey passes through manual recovery, identity confirmation, automatic
|
||||
completion, settings, and reconnect.
|
||||
|
||||
The full local Vitest run reported 16,080 passing tests, nine failing tests,
|
||||
and three suites with database-startup timeouts. Its failing suites are being
|
||||
rerun in isolation; this is not a claim of a green full suite. CI initially
|
||||
and three suites with database-startup timeouts. All ten failing suites passed
|
||||
in isolation (303 tests). The original full run remains a failed run; it is not
|
||||
a claim of a green full suite. CI initially
|
||||
identified outdated wizard expectations and missing API documentation, both now
|
||||
covered by the updated focused checks.
|
||||
covered by the updated focused checks. The full GitHub fixture also verifies
|
||||
installation permissions and preserved legacy tool authority. The browser
|
||||
report attaches screenshots of both wizard screens.
|
||||
|
||||
Cloud's final standard `npm test` passes: 2,559 tests and 73 environment-dependent
|
||||
Cloud's final standard `npm test` passes: 2,561 tests and 73 environment-dependent
|
||||
skips. Dedicated-App regressions cover normalized concurrent starts, stale
|
||||
installation redeliveries, atomic inbox/state transitions, current enrolled
|
||||
callback destinations, PKCE consent and denial, expired claims, and webhook
|
||||
secret retirement. The Postgres concurrency case requires a disposable
|
||||
callback destinations after old hosts stop resolving, PKCE consent and denial, expired claims, and webhook
|
||||
secret retirement with nonblocking cleanup, including overlapping repairs. The Postgres concurrency case requires a disposable
|
||||
database and was not run here.
|
||||
|
||||
Before rollout, apply the additive Cloud migration, enable the dedicated-App
|
||||
|
||||
@@ -8,7 +8,7 @@ import { githubChatManagementService } from "../services/chat-github-management.
|
||||
import { githubChatReviewService } from "../services/chat-github-reviews.js";
|
||||
import { githubReviewCheckService } from "../services/chat-github-checks.js";
|
||||
import { githubAutomaticReviewEvent } from "../services/chat-github-events.js";
|
||||
import { githubBotToolsForSession } from "../services/chat-github-tools.js";
|
||||
import { githubBotToolsForSession, syncGitHubBotTools } from "../services/chat-github-tools.js";
|
||||
import { resolveGitHubOperationCredentials } from "../services/github-operation-credentials.js";
|
||||
import { initializeRunIdentity } from "../services/run-identity.js";
|
||||
import { chatGitHubRegistrations, chatGitHubReviews, toolCatalogEntries } from "@paperclipai/db";
|
||||
@@ -1875,6 +1875,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
>
|
||||
> & { wakeup?: ChatChannelServiceOptions["heartbeat"]["wakeup"] } = {},
|
||||
useVerifiedAppId = false,
|
||||
registerWithManifest = false,
|
||||
) {
|
||||
let setupComplete = false;
|
||||
const deferredSchedule = overrides.scheduleDeferredWork;
|
||||
@@ -1891,16 +1892,21 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
},
|
||||
];
|
||||
let appPermissions: Record<string, string> = {
|
||||
contents: "read",
|
||||
checks: "write",
|
||||
issues: "write",
|
||||
metadata: "read",
|
||||
pull_requests: "write",
|
||||
};
|
||||
let installationPermissions: Record<string, string> = {
|
||||
contents: "read",
|
||||
checks: "write",
|
||||
issues: "write",
|
||||
metadata: "read",
|
||||
pull_requests: "write",
|
||||
};
|
||||
let appEvents = [
|
||||
"pull_request",
|
||||
"github_app_authorization",
|
||||
"installation",
|
||||
"installation_repositories",
|
||||
@@ -1959,6 +1965,8 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
{ status: 200, headers: { "content-type": "application/json" } },
|
||||
);
|
||||
}
|
||||
if (url === `https://api.github.com/app/installations/${installationId}`)
|
||||
return Response.json({ permissions: installationPermissions, suspended_at: null });
|
||||
if (url === "https://api.github.com/app/installations?per_page=100") {
|
||||
return new Response(
|
||||
JSON.stringify(
|
||||
@@ -2041,6 +2049,8 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
endpoint.publicId,
|
||||
webhookSecret,
|
||||
);
|
||||
if (registerWithManifest)
|
||||
await context.service.startGitHubRegistration(endpoint.id, "owner-user", "Maya");
|
||||
await context.service.configure(
|
||||
endpoint.id,
|
||||
{
|
||||
@@ -2052,10 +2062,17 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
},
|
||||
"owner-user",
|
||||
);
|
||||
if (!useVerifiedAppId) {
|
||||
const [configured] = await db.select().from(chatEndpoints).where(eq(chatEndpoints.id, endpoint.id));
|
||||
// Legacy transport fixtures already have explicit tool authority. Do not
|
||||
// save a review configuration: that would switch their admission model.
|
||||
await db.transaction((tx) => syncGitHubBotTools(tx, configured!, "owner-user", true));
|
||||
}
|
||||
const resources = await context.service.listResources(endpoint.id);
|
||||
await context.service.replaceResources(endpoint.id, [
|
||||
{ id: resources[0]!.id, enabled: true },
|
||||
]);
|
||||
if (!registerWithManifest)
|
||||
await context.service.replaceResources(endpoint.id, [
|
||||
{ id: resources[0]!.id, enabled: true },
|
||||
]);
|
||||
setupComplete = true;
|
||||
const callbacks = context.runtime.configurations.get(
|
||||
endpoint.id,
|
||||
@@ -2858,39 +2875,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
});
|
||||
it("imports initial GitHub access without a second picker and leaves later additions disabled", async () => {
|
||||
const f = await seedCompany();
|
||||
const context = await configuredGitHubEndpoint(f, {}, true);
|
||||
const context = await configuredGitHubEndpoint(f, {}, true, true);
|
||||
const current = await context.service.get(context.endpoint.id);
|
||||
const first = await context.service.listResources(current.id);
|
||||
// Inventory before any user selection: simulate a fresh provider import,
|
||||
// rather than an explicit user decision to disable every repository.
|
||||
await db
|
||||
.update(chatEndpointResources)
|
||||
.set({ enabled: false })
|
||||
.where(eq(chatEndpointResources.endpointId, current.id));
|
||||
await db
|
||||
.update(chatEndpoints)
|
||||
.set({
|
||||
setup: {
|
||||
...current.setup,
|
||||
github: {
|
||||
...current.setup.github,
|
||||
repositorySelectionSaved: undefined,
|
||||
initialRepositoryImportPending: true,
|
||||
},
|
||||
},
|
||||
})
|
||||
.where(eq(chatEndpoints.id, current.id));
|
||||
await db.insert(chatGitHubRegistrations).values({
|
||||
companyId: f.companyId,
|
||||
endpointId: current.id,
|
||||
userId: "owner-user",
|
||||
stateHash: randomUUID(),
|
||||
trustedOrigin: "https://reviews.example.test",
|
||||
ownerType: "organization",
|
||||
ownerLogin: "paperclipai",
|
||||
status: "completed",
|
||||
expiresAt: new Date(Date.now() + 60_000),
|
||||
});
|
||||
expect(current.setup.github?.initialRepositoryImportPending).toBe(true);
|
||||
await db.update(chatGitHubRegistrations).set({ status: "completed" })
|
||||
.where(eq(chatGitHubRegistrations.endpointId, current.id));
|
||||
const result = await context.service.githubWizard.advance(
|
||||
current.id,
|
||||
"owner-user",
|
||||
@@ -2978,6 +2967,20 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
);
|
||||
expect(f.wakeup).not.toHaveBeenCalled();
|
||||
});
|
||||
it("legacy name-only registration preserves an explicit empty repository selection", async () => {
|
||||
const fixture = await seedCompany();
|
||||
const { service } = createService(new FakeChatSdkRuntime(), fakeSlackFetch(), {
|
||||
publicBaseUrl: "https://reviews.example.test",
|
||||
});
|
||||
const bot = await service.create(fixture.companyId, {
|
||||
provider: "github", assignedAgentId: fixture.assignedAgentId,
|
||||
}, "owner-user");
|
||||
await service.replaceResources(bot.id, [], "owner-user");
|
||||
await service.startGitHubRegistration(bot.id, "owner-user", "Maya");
|
||||
const current = await service.get(bot.id);
|
||||
expect(current.setup.github?.repositorySelectionSaved).toBe(true);
|
||||
expect(current.setup.github?.initialRepositoryImportPending).not.toBe(true);
|
||||
});
|
||||
it("uses the manifest state parameter and rejects expired, reused, and wrong-origin registrations", async () => {
|
||||
const fixture = await seedCompany();
|
||||
const { service } = createService(
|
||||
|
||||
@@ -27963,7 +27963,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
) {
|
||||
const initial = await endpointRecord(endpointId);
|
||||
if (!initial) throw notFound("Chat endpoint not found");
|
||||
if (updates.length === 0) return listResources(endpointId);
|
||||
if (updates.length === 0 && initial.endpoint.provider !== "github") return listResources(endpointId);
|
||||
if (initial.endpoint.provider === "imessage-photon" && initial.endpoint.botExternalId?.startsWith("photon-project:") && updates.some((entry) => entry.enabled))
|
||||
throw unprocessable("Photon shared channels support direct messages only; groups cannot be enabled");
|
||||
await withCredentialMutationLease(
|
||||
@@ -28045,7 +28045,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
);
|
||||
if (endpoint.provider === "github")
|
||||
await tx.update(chatEndpoints).set({ setup: sql`jsonb_set(${chatEndpoints.setup}, '{github}', coalesce(${chatEndpoints.setup}->'github', '{}'::jsonb) || '{"repositorySelectionSaved":true,"initialRepositoryImportPending":false}'::jsonb)`, updatedAt: new Date() }).where(eq(chatEndpoints.id, endpointId));
|
||||
if (changes.length > 0)
|
||||
if (changes.length > 0 || (endpoint.provider === "github" && !endpoint.setup.github?.repositorySelectionSaved))
|
||||
await logActivity(
|
||||
tx as unknown as Db,
|
||||
{
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import { createHash, randomBytes } from "node:crypto";
|
||||
import { and, eq, gt } from "drizzle-orm";
|
||||
import { and, eq, gt, sql } from "drizzle-orm";
|
||||
import {
|
||||
chatEndpoints,
|
||||
chatEndpointResources,
|
||||
chatGitHubRegistrations,
|
||||
companies,
|
||||
companyMemberships,
|
||||
@@ -111,11 +112,25 @@ export function githubChatRegistrationService(
|
||||
const state = randomBytes(32).toString("base64url");
|
||||
const expiresAt = new Date(Date.now() + 30 * 60_000);
|
||||
await db.transaction(async (tx) => {
|
||||
await tx
|
||||
.select({ id: chatEndpoints.id })
|
||||
const [lockedEndpoint] = await tx
|
||||
.select()
|
||||
.from(chatEndpoints)
|
||||
.where(eq(chatEndpoints.id, endpointId))
|
||||
.for("update");
|
||||
if (!lockedEndpoint || lockedEndpoint.status === "archived")
|
||||
throw notFound("GitHub bot not found");
|
||||
if (lockedEndpoint.botExternalId)
|
||||
throw conflict("This bot already has a GitHub App. Reconnect its existing credentials.");
|
||||
const [existingResource] = await tx
|
||||
.select({ id: chatEndpointResources.id })
|
||||
.from(chatEndpointResources)
|
||||
.where(eq(chatEndpointResources.endpointId, endpointId))
|
||||
.limit(1);
|
||||
if (!lockedEndpoint.setup.github?.repositorySelectionSaved && !existingResource)
|
||||
await tx.update(chatEndpoints).set({
|
||||
setup: sql`jsonb_set(${chatEndpoints.setup}, '{github}', coalesce(${chatEndpoints.setup}->'github', '{}'::jsonb) || '{"initialRepositoryImportPending":true}'::jsonb)`,
|
||||
updatedAt: new Date(),
|
||||
}).where(eq(chatEndpoints.id, endpointId));
|
||||
await tx
|
||||
.update(chatGitHubRegistrations)
|
||||
.set({ status: "failed", consumedAt: new Date() })
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { readFile } from "node:fs/promises";
|
||||
import { expect, test, type Page } from "@playwright/test";
|
||||
import { expect, test, type Page, type TestInfo } from "@playwright/test";
|
||||
|
||||
import {
|
||||
expectMinimumProviderSetup,
|
||||
@@ -18,12 +18,13 @@ import {
|
||||
GITHUB_PRIVATE_KEY_PASTE_FIXTURE,
|
||||
} from "./chat-adapters-ui.shared";
|
||||
|
||||
async function exerciseGitHubReviewSetup(page: Page, mock: ChatMock, seed: Seed, provider: ProviderCase) {
|
||||
async function exerciseGitHubReviewSetup(page: Page, mock: ChatMock, seed: Seed, provider: ProviderCase, testInfo: TestInfo) {
|
||||
await expect(page.getByRole("heading", { name: "Choose agent", exact: true })).toBeVisible();
|
||||
await page.getByRole("button", { name: "Select agent…", exact: true }).click();
|
||||
await page.getByRole("button", { name: "Select Maya", exact: true }).click();
|
||||
await expect(page.getByText("Maya is not configured for low-trust review")).toBeVisible();
|
||||
await expect(page.getByRole("link", { name: "Learn about low-trust agents" })).toHaveAttribute("href", /trust-and-low-trust-review/);
|
||||
await testInfo.attach("Choose agent — inline low-trust warning", { body: await page.screenshot(), contentType: "image/png" });
|
||||
await page.getByRole("button", { name: "Change Maya to a low trust agent" }).click();
|
||||
await expect(page.getByText("Maya is not configured for low-trust review")).toHaveCount(0);
|
||||
await page.getByRole("button", { name: "Continue", exact: true }).click();
|
||||
@@ -33,6 +34,7 @@ async function exerciseGitHubReviewSetup(page: Page, mock: ChatMock, seed: Seed,
|
||||
await expect(page.getByLabel("App name", { exact: true })).toHaveValue("Maya");
|
||||
await page.getByLabel("GitHub account").selectOption("organization");
|
||||
await page.getByLabel("Organization", { exact: true }).fill("paperclip-ai");
|
||||
await testInfo.attach("Connect GitHub — organization", { body: await page.screenshot(), contentType: "image/png" });
|
||||
await page.getByRole("button", { name: "Save & exit", exact: true }).click();
|
||||
await page.goto(`/${seed.prefix}/apps/chat/connect?provider=github&purpose=chat&resume=endpoint-github`);
|
||||
await expect(page.getByLabel("GitHub account")).toHaveValue("organization");
|
||||
@@ -204,7 +206,7 @@ test.describe.serial("native chat adapter UI", () => {
|
||||
for (const provider of PROVIDERS) {
|
||||
test(`${provider.name}: catalog, setup, and connection management tabs`, async ({
|
||||
page,
|
||||
}) => {
|
||||
}, testInfo) => {
|
||||
const mock = await installChatControlPlaneMock(page, provider, seed, {
|
||||
enableChatConnectors: true,
|
||||
});
|
||||
@@ -268,7 +270,7 @@ test.describe.serial("native chat adapter UI", () => {
|
||||
}
|
||||
|
||||
if (provider.provider === "github") {
|
||||
await exerciseGitHubReviewSetup(page, mock, seed, provider);
|
||||
await exerciseGitHubReviewSetup(page, mock, seed, provider, testInfo);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user