mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
## Thinking Path > - Paperclip connects external work to an agent and its ordinary tasks. > - A dedicated GitHub App must prove signed webhook delivery during setup. > - Cloud queues the original bytes until the tenant can authenticate them. > - An older personal connection can leave a long empty-queue backoff in that tenant. > - Active App setup can then time out before the queued ping is read. > - This change lets active setup request the existing event drain with a short rate limit. > - The same webhook handler still owns signature verification and setup state. ## Linked Issues or Issue Description Refs: #17457, #17513. **What happened?** Cloud accepted a new App's ping, but the tenant read it only after the setup wait timed out and cleanup archived the draft. An existing personal GitHub connection had left an empty-queue backoff. The poller checked that deadline before it read the new App state. **Expected behavior** An active manager setup request must be able to read the queued ping promptly. It must not bypass signature checks or create another App. **Steps to reproduce** 1. Let a personal GitHub connection build its empty-queue backoff. 2. Start a dedicated App and complete its installation. 3. Return to the waiting setup screen while the signed ping is queued. **Paperclip version or commit** This behavior is present on the base commit of this PR. ## What Changed - Let the authenticated wizard request a queue drain only while its Cloud App still lacks signed-delivery verification. Existing manager, credential, and installation checks run first. - Register a narrow callback to the server-owned GitHub poller. Keep provider handling in the GitHub services. Remove the callback on server close. - Share one in-flight attempt between setup and background requests. Limit setup attempts to one per five seconds, including after errors. - Retain the existing passive backoff, sealed queue, acknowledgement rules, and canonical webhook signature handler. - Add regressions for the old backoff, concurrent requests, failed requests, and wizard signature verification. Update the setup documentation and map. ## Verification - Observed the backoff regression fail against the old service: active setup made no additional Cloud lease request. It passes with this fix. - Three focused event-poller tests pass. They use the real admission/backoff code with stubbed SQL and Cloud boundaries. - All 39 startup tests pass. Server TypeScript, map checks, and diff checks pass. - The Postgres integration suites could not start locally because this host exhausted its semaphore limit. They remain required CI checks. This is not a passing local integration result. - Full current-head CI passes, including the Postgres and chat integration suites. Greptile scored this exact head 5/5 with no unresolved findings. - The additional Paperclip review check ended neutral because its execution did not complete. It did not publish a validated assessment. - A fresh deployed setup attempt remains required before live qualification. ## Risks A waiting setup screen now makes a bounded readiness request to Cloud. The existing connector request timeout still applies. Concurrent calls share the same drain, and failures release it for a later retry. The request stops after persisted verification or when the browser stops advancing setup. No timer, durable queue, fleet setting, or sleep blocker is added or removed. The existing background reconciliation path still has its existing sleep restrictions. Live setup-to-review behavior is not established by these local tests. ## Model Used OpenAI Codex, based on GPT-6. The exact runtime model ID and context-window size are not exposed in this session. Used for code execution and regression analysis. ## 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 <paperclip@paperclip.ing>
75 lines
3.6 KiB
TypeScript
75 lines
3.6 KiB
TypeScript
import { describe, expect, it, vi } from "vitest";
|
|
import { chatEndpoints, type Db } from "@paperclipai/db";
|
|
import { githubConnectionEventService } from "./github-connection-events.js";
|
|
import type { PaperclipCloudConnector } from "./paperclip-cloud-connector.js";
|
|
|
|
function fixture() {
|
|
let time = Date.parse("2026-09-04T12:00:00Z");
|
|
const bindings = [{
|
|
grant: { id: "grant", companyId: "company", kind: "user", subjectUserId: "owner", status: "active",
|
|
providerTenant: { github: { installationIds: ["101"] } } },
|
|
connection: { id: "connection", config: { sourceTemplateKey: "github", oauth: { connectorProfile: "github.code" } } },
|
|
}];
|
|
const bots: Array<{ companyId: string }> = [];
|
|
// Only SQL is stubbed. Backoff, lease admission and concurrent execution use
|
|
// the real service; relational persistence has its own Postgres suite.
|
|
const select = vi.fn(() => ({ from: (table: unknown) => table === chatEndpoints
|
|
? { where: async () => bots }
|
|
: { innerJoin: () => ({ where: async () => bindings }) } }));
|
|
const leaseEvents = vi.fn<PaperclipCloudConnector["leaseEvents"]>().mockResolvedValue(null);
|
|
const connector = { leaseEvents } as unknown as PaperclipCloudConnector;
|
|
const service = githubConnectionEventService({ select } as unknown as Db, { connector, now: () => new Date(time) });
|
|
return { service, leaseEvents, select, bots, advance: (ms: number) => { time += ms; } };
|
|
}
|
|
|
|
describe("GitHub event polling during active App setup", () => {
|
|
it("reaches Cloud during an old empty-queue backoff without expanding idle polling", async () => {
|
|
const f = fixture();
|
|
for (const delay of [0, 10_000, 20_000, 40_000, 80_000]) {
|
|
f.advance(delay);
|
|
await f.service.pollOnce();
|
|
}
|
|
expect(f.leaseEvents).toHaveBeenCalledTimes(5);
|
|
const queries = f.select.mock.calls.length;
|
|
f.bots.push({ companyId: "company" });
|
|
f.advance(5_000);
|
|
for (let index = 0; index < 3; index++) await f.service.pollOnce();
|
|
expect(f.select).toHaveBeenCalledTimes(queries);
|
|
expect(f.leaseEvents).toHaveBeenCalledTimes(5);
|
|
await f.service.pollOnce({ forSetup: true });
|
|
expect(f.leaseEvents).toHaveBeenCalledTimes(6);
|
|
const setupQueries = f.select.mock.calls.length;
|
|
await f.service.pollOnce({ forSetup: true });
|
|
expect(f.leaseEvents).toHaveBeenCalledTimes(6);
|
|
expect(f.select).toHaveBeenCalledTimes(setupQueries);
|
|
f.advance(5_000);
|
|
await f.service.pollOnce({ forSetup: true });
|
|
expect(f.leaseEvents).toHaveBeenCalledTimes(7);
|
|
});
|
|
|
|
it("shares one lease between setup and scheduled callers while Cloud is slow", async () => {
|
|
const f = fixture();
|
|
let complete!: (value: null) => void;
|
|
f.leaseEvents.mockImplementationOnce(() => new Promise(resolve => { complete = resolve; }));
|
|
const scheduled = f.service.pollOnce();
|
|
const setup = f.service.pollOnce({ forSetup: true });
|
|
await vi.waitFor(() => expect(f.leaseEvents).toHaveBeenCalledTimes(1));
|
|
expect(complete).toBeTypeOf("function");
|
|
complete(null);
|
|
await Promise.all([scheduled, setup]);
|
|
expect(f.leaseEvents).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("releases the shared attempt after an error and retries only after the setup throttle", async () => {
|
|
const f = fixture();
|
|
f.leaseEvents.mockRejectedValueOnce(new Error("Cloud unavailable"));
|
|
await expect(f.service.pollOnce({ forSetup: true })).rejects.toThrow("Cloud unavailable");
|
|
await f.service.pollOnce({ forSetup: true });
|
|
await f.service.pollOnce();
|
|
expect(f.leaseEvents).toHaveBeenCalledTimes(1);
|
|
f.advance(5_000);
|
|
await f.service.pollOnce({ forSetup: true });
|
|
expect(f.leaseEvents).toHaveBeenCalledTimes(2);
|
|
});
|
|
});
|