mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 04:02:18 +01:00
## Thinking Path > - Paperclip manages AI agents and their access to external tools. > - Tool connections discover repositories through GitHub account grants. > - The tool-access service mixes GitHub response parsing with grant authorization and persistence. > - These responsibilities have different inputs and failure rules. > - This PR puts GitHub discovery in one provider module. > - The service keeps current authority checks and stored grant updates. ## Linked Issues or Issue Description **What existing behavior does this improve?** The ownership and review of GitHub access discovery in the tool-access service. **Current behavior** The 21,525-line service contains GitHub pagination, account metadata validation, and grant lifecycle code. **Proposed behavior** A GitHub module returns normalized access metadata. Existing service exports remain compatible. This is the first PR in a three-part refactor stack. No behavior change is intended. A search found no existing tool-access refactor PR or issue. Stack order: this PR, #17146, then #17147. Each PR targets its predecessor. ## What Changed - Move six functions into `tool-access/github-access-discovery.ts`. Their bodies are unchanged. - Keep grant selection, credential resolution, refresh retry, membership locks, revision checks, and webhook updates in the service. - Add public-entry characterization tests for HTTP error classification and invalid pagination. - Record five candidate boundaries and the initial three-PR scope in the dated plan. - Reduce `tool-access.ts` from 21,525 lines / 829,519 bytes to 21,237 lines / 820,116 bytes. The new module is 294 lines / 9,722 bytes. ## Verification - Before and after: `pnpm exec vitest run server/src/__tests__/github-grant-metadata.test.ts server/src/__tests__/project-repositories.test.ts` — 13 tests pass. - Before and after: `pnpm exec vitest run server/src/__tests__/tool-access-service.test.ts -t 'GitHub|github'` — 11 selected tests pass with disposable PostgreSQL; 455 cases are outside this selection. - `pnpm --filter @paperclipai/server typecheck` passes. - Exact text comparison confirms all six moved bodies match the base. `git diff --check` passes. - The initial baseline attempt failed because the plugin SDK had not been built. The normal workspace prerequisite build resolved it before behavioral validation. - Final-stack local verification: `pnpm -r --workspace-concurrency=2 typecheck` and `pnpm build` pass. CI is green at `07e78dd1b1c26e0bd4234006766444c71e234451`. Greptile is 5/5 with no actionable findings. - Full local-suite limitation: `pnpm test:run` was interrupted after 79 minutes during the serial general-server phase, after CI had completed the full test matrix. Before interruption, it flagged the unchanged `company-search-service.test.ts` suite (26 database cases skipped). One isolated rerun passed all 28 cases without code changes. The original log and a pre-interruption copy are preserved. The original failure cause was not reproduced. The full local suite and its later groups are not claimed as passed; the complete CI matrix is green on this stack. ## Risks - Import wiring could break existing consumers. Existing imports remain supported and all four production call sites are unchanged. - Provider requests retain their ordering, timeout, fixed GitHub origin, and response redaction. The new module has no database dependency or shared mutable state. - Tests simulate GitHub responses. They do not qualify live GitHub access. ## Model Used OpenAI Codex, GPT-6 family, with code execution and repository tools. The exact serving model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
119 lines
5.9 KiB
TypeScript
119 lines
5.9 KiB
TypeScript
import { describe, expect, it, vi } from "vitest";
|
|
import { loadGitHubGrantMetadata } from "../services/tool-access.js";
|
|
|
|
function json(value: unknown, next = false): Response {
|
|
return new Response(JSON.stringify(value), {
|
|
headers: {
|
|
"content-type": "application/json",
|
|
...(next ? { link: '<https://api.github.com/next>; rel="next"' } : {}),
|
|
},
|
|
});
|
|
}
|
|
|
|
describe("GitHub grant metadata", () => {
|
|
it("lists every page of installations and repositories, persisting only display metadata", async () => {
|
|
const request = vi.fn<typeof fetch>(async (input) => {
|
|
const url = new URL(String(input));
|
|
const secondPage = url.searchParams.get("page") === "2";
|
|
if (url.pathname === "/user") return json({ id: 42, login: "octocat", avatar_url: "https://avatars.example/octocat" });
|
|
if (url.pathname === "/user/installations") {
|
|
return json({ installations: secondPage
|
|
? [{ id: 102, repository_selection: "all", account: { login: "octocat" } }]
|
|
: [{ id: 101, repository_selection: "selected", html_url: "https://github.com/settings/installations/101", account: { login: "paperclipai" } }],
|
|
}, !secondPage);
|
|
}
|
|
if (url.pathname === "/user/installations/101/repositories") {
|
|
return json({ total_count: 2, repositories: secondPage
|
|
? [{ id: 2, full_name: "paperclipai/b", owner: { type: "Organization" }, private: true, description: "must-not-persist", clone_url: "must-not-persist" }]
|
|
: [{ id: 1, full_name: "paperclipai/a", owner: { type: "Organization" }, private: false }],
|
|
}, !secondPage);
|
|
}
|
|
if (url.pathname === "/user/installations/102/repositories") {
|
|
return json({ total_count: 1, repositories: [{ id: 3, full_name: "octocat/c", owner: { type: "User" } }] });
|
|
}
|
|
throw new Error(`Unexpected GitHub path: ${url.pathname}`);
|
|
});
|
|
|
|
const metadata = await loadGitHubGrantMetadata("ghu_secret", request, "paperclip-development");
|
|
expect(metadata).toMatchObject({
|
|
userId: "42",
|
|
login: "octocat",
|
|
installationCount: 2,
|
|
repositoryCount: 3,
|
|
repositorySelection: "mixed",
|
|
installationIds: ["101", "102"],
|
|
installationOwnerLogins: ["paperclipai", "octocat"],
|
|
repositories: [
|
|
{ id: "3", fullName: "octocat/c", installationId: "102", ownerType: "personal" },
|
|
{ id: "1", fullName: "paperclipai/a", installationId: "101", ownerType: "organization", private: false },
|
|
{ id: "2", fullName: "paperclipai/b", installationId: "101", ownerType: "organization", private: true },
|
|
],
|
|
installationUrl: "https://github.com/apps/paperclip-development/installations/new",
|
|
managementUrl: "https://github.com/settings/installations/101",
|
|
appSlug: "paperclip-development",
|
|
webhookHealth: "pending",
|
|
});
|
|
expect(metadata.repositories[0]).not.toHaveProperty("private");
|
|
expect(JSON.stringify(metadata)).not.toContain("must-not-persist");
|
|
expect(request).toHaveBeenCalledTimes(6);
|
|
for (const [input, init] of request.mock.calls) {
|
|
expect(new URL(String(input)).origin).toBe("https://api.github.com");
|
|
expect(new Headers(init?.headers).get("authorization")).toBe("Bearer ghu_secret");
|
|
}
|
|
});
|
|
|
|
it("recovers a legacy grant's app chooser from GitHub installation metadata", async () => {
|
|
const request = vi.fn<typeof fetch>()
|
|
.mockResolvedValueOnce(json({ id: 42, login: "octocat" }))
|
|
.mockResolvedValueOnce(json({ installations: [{ id: 101, app_slug: "paperclip-staging", repository_selection: "selected" }] }))
|
|
.mockResolvedValueOnce(json({ repositories: [{ id: 1, full_name: "octocat/a" }] }));
|
|
await expect(loadGitHubGrantMetadata("ghu_secret", request)).resolves.toMatchObject({
|
|
appSlug: "paperclip-staging",
|
|
installationUrl: "https://github.com/apps/paperclip-staging/installations/new",
|
|
});
|
|
});
|
|
|
|
it("requires at least one installation with an accessible repository", async () => {
|
|
const request = vi.fn<typeof fetch>(async (input) => String(input).endsWith("/user")
|
|
? json({ id: 42, login: "octocat" })
|
|
: json({ installations: [] }));
|
|
await expect(loadGitHubGrantMetadata("ghu_secret", request)).rejects.toMatchObject({
|
|
details: expect.objectContaining({ code: "github_installation_required" }),
|
|
});
|
|
});
|
|
|
|
it("does not report a partial repository list when a later page fails", async () => {
|
|
const request = vi.fn<typeof fetch>()
|
|
.mockResolvedValueOnce(json({ id: 42, login: "octocat" }))
|
|
.mockResolvedValueOnce(json({ installations: [{ id: 101, repository_selection: "selected" }] }))
|
|
.mockResolvedValueOnce(json({ repositories: [{ id: 1, full_name: "octocat/a" }] }, true))
|
|
.mockResolvedValueOnce(new Response("Unavailable", { status: 503 }));
|
|
await expect(loadGitHubGrantMetadata("ghu_secret", request)).rejects.toMatchObject({
|
|
details: expect.objectContaining({ code: "github_access_check_failed" }),
|
|
});
|
|
});
|
|
|
|
it.each([
|
|
[401, "oauth_reauthorization_required"],
|
|
[503, "github_access_check_failed"],
|
|
])("preserves the retry decision for HTTP %s without reflecting provider text", async (status, code) => {
|
|
const request = vi.fn<typeof fetch>().mockResolvedValue(new Response("private provider detail", { status }));
|
|
await expect(loadGitHubGrantMetadata("ghu_secret", request)).rejects.toMatchObject({
|
|
status: 422,
|
|
message: "GitHub could not verify this account's installation access",
|
|
details: { code },
|
|
});
|
|
expect(request).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("rejects an empty next page before reporting an incomplete access snapshot", async () => {
|
|
const request = vi.fn<typeof fetch>()
|
|
.mockResolvedValueOnce(json({ id: 42, login: "octocat" }))
|
|
.mockResolvedValueOnce(json({ installations: [] }, true));
|
|
await expect(loadGitHubGrantMetadata("ghu_secret", request)).rejects.toMatchObject({
|
|
details: { code: "github_bad_response" },
|
|
});
|
|
expect(request).toHaveBeenCalledTimes(2);
|
|
});
|
|
});
|