fix(github): fence retry attempts and restored binding intent

Keep provider backoff tied to the credential revision actually attempted. Rebuild restoration debt and exclude active installations from retirement commands.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-10-30 21:22:18 -05:00
1 parent a5c1e2533a
commit 9e1694211d
4 files changed
+144 -15

No files matched your search

@@ -43,7 +43,11 @@ A grant stores a unique `bindingSync` revision and removed installation IDs befo
Cloud registration calls. Successful delivery clears only the locked current
revision. Failure retains the saved intent and an exponential retry deadline,
including provider Retry-After advice. A restart loads that deadline from the
grant. Binding-only retries use the existing credential refresh lease and current
grant. A retry write compares the attempted credential/installation revision under
the grant lock and cannot delay work committed by a newer revision. Restoration
creates a new binding revision and removes active installations from the pending
retirement list; binding commands also reject legacy conflicting retirements.
Binding-only retries use the existing credential refresh lease and current
secret resolver without repeating account/repository discovery. The initial OAuth
callback uses the same saved debt and settlement path after connection activation.
@@ -6,9 +6,9 @@ import { createServer, type Server } from "node:http";
import { once } from "node:events";
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import { eq, sql } from "drizzle-orm";
import { applyPendingMigrations, companies, activityLog, companySecrets, connectionGrants, createDb, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS, toolApplications, toolConnections } from "@paperclipai/db";
import { applyPendingMigrations, companies, activityLog, companySecrets, connectionEventDeliveries, connectionGrants, createDb, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS, toolApplications, toolConnections } from "@paperclipai/db";
import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js";
import { createGitHubGrantContinuity, githubGrantContinuityDeadline } from "../services/tool-access/github-grant-continuity.js";
import { createGitHubGrantContinuity, githubGrantContinuityDeadline, githubContinuityRevision } from "../services/tool-access/github-grant-continuity.js";
import { createPaperclipCloudConnector } from "../services/paperclip-cloud-connector.js";
import { githubConnectionEventService } from "../services/github-connection-events.js";
import { createDeliveryWorkCoordinator } from "../services/delivery-work-coordinator.js";
@@ -36,6 +36,7 @@ describeDatabase("GitHub coordinator with PostgreSQL and controlled Cloud HTTP (
const signing = generateKeyPairSync("ed25519"), sealing = generateKeyPairSync("x25519");
let releaseRefresh: (() => void) | undefined, refreshArrived: (() => void) | undefined;
let refreshGate: Promise<void> | undefined;
let discoveryFailureGate: Promise<void> | undefined, discoveryArrived: (() => void) | undefined;
let heldBindingCompany: string | undefined, bindingGate: Promise<void> | undefined;
let bindingArrived: (() => void) | undefined, bindingClosed: (() => void) | undefined;
const realFetch = globalThis.fetch;
@@ -86,6 +87,10 @@ describeDatabase("GitHub coordinator with PostgreSQL and controlled Cloud HTTP (
res.writeHead(failBindings ? 503 : 200, { "content-type": "application/json" }).end("{}"); return;
}
githubRequests++;
if (req.url === "/user" && discoveryFailureGate) {
discoveryArrived?.(); await discoveryFailureGate;
res.writeHead(503, { "content-type": "application/json", "retry-after": "86400" }).end("{}"); return;
}
const body = req.url === "/user" ? { id: 42, login: "octocat" }
: req.url!.startsWith("/user/installations?") ? { installations: [{ id: 101, repository_selection: "all", account: { login: "octocat" } }] }
: { total_count: 1, repositories: [{ id: 999, full_name: "octocat/repo" }] };
@@ -187,6 +192,108 @@ describeDatabase("GitHub coordinator with PostgreSQL and controlled Cloud HTTP (
expect(await restarted.nextGitHubConnectionContinuityAt()).toBeNull();
});
it.each(["credential", "installation"])("does not attach a withheld provider failure to newer %s work", async change => {
const f = await fixture(["101"]);
vi.spyOn(globalThis, "fetch").mockImplementation((url, init) => String(url).startsWith("https://api.github.com")
? realFetch(`${origin}${new URL(String(url)).pathname}${new URL(String(url)).search}`, init) : realFetch(url, init));
await db.update(connectionGrants).set({ providerTenant: { ...f.grant.providerTenant,
oauth: { ...f.grant.providerTenant!.oauth, accessTokenExpiresAt: new Date(Date.now() + 30 * 60_000).toISOString() } } }).where(eq(connectionGrants.id, f.grant.id));
let release!: () => void;
const arrived = new Promise<void>(resolve => { discoveryArrived = resolve; });
discoveryFailureGate = new Promise<void>(resolve => { release = resolve; });
const pending = f.service.sweepGitHubConnectionContinuity(f.company.id);
const finished = Promise.allSettled([pending]);
try {
await Promise.race([arrived, pending.then(() => { throw new Error("Expected held provider request"); })]);
const revision = randomUUID();
const expiry = new Date(Date.now() + 10 * 60_000).toISOString();
await db.transaction(async tx => {
await notifyGitHubCredentialChange(tx, f.connection);
await tx.update(connectionGrants).set({ providerTenant: change === "credential"
? sql`jsonb_set(${connectionGrants.providerTenant}, '{oauth,accessTokenExpiresAt}', ${JSON.stringify(expiry)}::jsonb)`
: sql`jsonb_set(${connectionGrants.providerTenant}, '{github,bindingSync}', ${JSON.stringify({ id: revision, removeInstallationIds: ["202"] })}::jsonb)`,
updatedAt: new Date() }).where(eq(connectionGrants.id, f.grant.id));
});
release();
expect(await pending).toMatchObject({ due: 1, failed: 1 });
const [saved] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, f.grant.id));
expect(saved!.providerTenant!.github!.continuityRetry).toBeUndefined();
if (change === "credential") expect(saved!.providerTenant!.oauth!.accessTokenExpiresAt).toBe(expiry);
else expect(saved!.providerTenant!.github!.bindingSync!.id).toBe(revision);
expect(githubGrantContinuityDeadline(f.connection, saved!)).toBeLessThanOrEqual(Date.now());
discoveryFailureGate = undefined;
const restarted = createGitHubGrantContinuity(db, f.dependencies);
expect(await restarted.sweepGitHubConnectionContinuity(f.company.id)).toMatchObject({ due: 1, refreshed: 1, failed: 0 });
expect(bindings).toContainEqual(expect.objectContaining({ installationId: "101", active: true }));
if (change === "installation") expect(bindings).toContainEqual(expect.objectContaining({ installationId: "202", active: false }));
const [settled] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, f.grant.id));
expect(settled!.providerTenant!.github!.bindingSync).toBeUndefined();
const next = await restarted.nextGitHubConnectionContinuityAt(f.company.id);
expect(next).toBeLessThan(Date.now() + 24 * 60 * 60_000);
} finally { release(); await finished; discoveryFailureGate = undefined; discoveryArrived = undefined; }
});
it("retains provider backoff for the actual attempted revision after its own credential rotation", async () => {
const f = await fixture(["101"]);
await db.update(connectionGrants).set({ providerTenant: { ...f.grant.providerTenant,
oauth: { ...f.grant.providerTenant!.oauth, accessTokenExpiresAt: new Date(Date.now() + 30 * 60_000).toISOString() } } }).where(eq(connectionGrants.id, f.grant.id));
vi.spyOn(globalThis, "fetch").mockImplementation((url, init) => String(url).startsWith("https://api.github.com")
? realFetch(`${origin}${new URL(String(url)).pathname}${new URL(String(url)).search}`, init) : realFetch(url, init));
const owner = createGitHubGrantContinuity(db, { ...f.dependencies, refreshOAuthGrantCredentials: async () => {
const [rotated] = await db.update(connectionGrants).set({ providerTenant: sql`jsonb_set(${connectionGrants.providerTenant}, '{oauth}', ${JSON.stringify({
accessTokenExpiresAt: new Date(Date.now() + 10 * 60_000).toISOString(), refreshedAt: new Date().toISOString(),
})}::jsonb)` }).where(eq(connectionGrants.id, f.grant.id)).returning();
return rotated!;
} });
discoveryFailureGate = Promise.resolve();
try {
expect(await owner.sweepGitHubConnectionContinuity(f.company.id)).toMatchObject({ due: 1, failed: 1 });
const [saved] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, f.grant.id));
expect(saved!.providerTenant!.github!.continuityRetry!.revision).toBe(githubContinuityRevision(saved!));
expect(await owner.nextGitHubConnectionContinuityAt(f.company.id)).toBeGreaterThan(Date.now() + 23 * 60 * 60_000);
const calls = githubRequests;
expect(await createGitHubGrantContinuity(db, f.dependencies).sweepGitHubConnectionContinuity(f.company.id)).toMatchObject({ due: 0 });
expect(githubRequests).toBe(calls);
} finally { discoveryFailureGate = undefined; }
});
it.each([["suspend", "unsuspend"], ["unsuspend", "suspend"]])("reconciles same-batch installation order %s then %s and duplicate replay", async (first, last) => {
const f = await fixture(["101"]);
await db.update(connectionGrants).set({ providerTenant: { ...f.grant.providerTenant,
github: { ...f.grant.providerTenant!.github!, bindingSync: { id: randomUUID(), removeInstallationIds: ["202"] } } } }).where(eq(connectionGrants.id, f.grant.id));
const events = [first, last].map(action => ({ id: randomUUID(), provider: "github" as const, event: "installation", action,
installationId: "101", repositoryId: null, createdAt: new Date(Date.now() + 1000).toISOString(), bindingIds: [`${f.grant.id}_101`], payload: { installationId: "101" } }));
const acknowledge = vi.fn(async () => events.length);
const ingress = githubConnectionEventService(db, { connector: { ...f.connector, leaseEvents: async () => ({ leaseId: "installation-batch", events }), acknowledgeEvents: acknowledge } });
expect(await ingress.pollOnce({ forDelivery: true })).toMatchObject({ processed: 2, failed: 0 });
const [saved] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, f.grant.id));
const active = last === "unsuspend";
expect(saved!.providerTenant!.github!.installationIds).toEqual(active ? ["101"] : []);
expect(saved!.providerTenant!.github!.bindingSync!.removeInstallationIds.sort()).toEqual(active ? ["202"] : ["101", "202"]);
expect(githubGrantContinuityDeadline(f.connection, saved!)).toBe(0);
await f.service.syncGitHubGrantBindings(f.connection, saved!);
expect(bindings.filter(binding => binding.installationId === "101")).toEqual([expect.objectContaining({ active })]);
expect(bindings.filter(binding => binding.installationId === "202")).toEqual([expect.objectContaining({ active: false })]);
const [settled] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, f.grant.id));
expect(settled!.providerTenant!.github!.bindingSync).toBeUndefined();
// Batch duplicates must retain terminal receipts and emit no registration changes.
expect(await ingress.pollOnce({ forDelivery: true })).toMatchObject({ processed: 0, duplicate: active ? 2 : 0, ignored: active ? 0 : 2, failed: 0 });
await f.service.syncGitHubGrantBindings(f.connection, saved!);
expect(bindings).toHaveLength(2);
expect((await db.select().from(connectionGrants).where(eq(connectionGrants.id, f.grant.id)))[0]!.providerTenant).toEqual(settled!.providerTenant);
expect((await db.select().from(connectionEventDeliveries).where(eq(connectionEventDeliveries.companyId, f.company.id))).map(receipt => receipt.status)).toEqual(["processed", "processed"]);
expect(acknowledge).toHaveBeenCalledTimes(2);
});
it("does not retire active installations from a legacy conflicting binding intent", async () => {
const f = await fixture(["101"]);
const [saved] = await db.update(connectionGrants).set({ providerTenant: { ...f.grant.providerTenant,
github: { ...f.grant.providerTenant!.github!, bindingSync: { id: randomUUID(), removeInstallationIds: ["101", "202"] } } } }).where(eq(connectionGrants.id, f.grant.id)).returning();
await f.service.syncGitHubGrantBindings(f.connection, saved!);
expect(bindings.filter(binding => binding.installationId === "101")).toEqual([expect.objectContaining({ active: true })]);
expect(bindings.filter(binding => binding.installationId === "202")).toEqual([expect.objectContaining({ active: false })]);
});
it("preserves webhook removal debt across an actual signed, held Cloud token rotation and lease cleanup", async () => {
const f = await fixture(["101"]);
vi.stubEnv("PAPERCLIP_SECRETS_MASTER_KEY", "01234567890123456789012345678901");
@@ -327,6 +434,11 @@ describeDatabase("GitHub coordinator with PostgreSQL and controlled Cloud HTTP (
expect(leases).toBeLessThanOrEqual(3); expect(load.mock.calls.length).toBeLessThanOrEqual(2);
if (loadTimes.length === 2) expect(loadTimes[1]! - loadTimes[0]!).toBeGreaterThanOrEqual(5_000);
if (leaseTimes.length === 3) expect(leaseTimes[2]! - leaseTimes[1]!).toBeGreaterThanOrEqual(5_000);
// A hint arriving during the first SQL pass retains one spaced generation.
// Start the empty-window assertion after that concrete work has settled.
await vi.advanceTimersByTimeAsync(31_000); await settled(coordinator);
expect(leases).toBeLessThanOrEqual(3); expect(load.mock.calls.length).toBeLessThanOrEqual(2);
if (loadTimes.length === 2) expect(loadTimes[1]! - loadTimes[0]!).toBeGreaterThanOrEqual(5_000);
const quiet = { leases, loads: load.mock.calls.length };
await vi.advanceTimersByTimeAsync(60_000);
expect({ leases, loads: load.mock.calls.length }).toEqual(quiet);
@@ -330,9 +330,9 @@ export function githubConnectionEventService(
// Lifecycle webhooks carry IDs, not the user token’s complete repository view.
// Discard the snapshot until Refresh access verifies it again.
accessRevision: randomUUID(),
...(unavailable ? { bindingSync: {
...(unavailable || github.bindingSync || !github.installationIds.includes(binding.installationId) ? { bindingSync: {
id: randomUUID(), removeInstallationIds: [...new Set([
...(github.bindingSync?.removeInstallationIds ?? []), binding.installationId,
...(github.bindingSync?.removeInstallationIds ?? []), ...(unavailable ? [binding.installationId] : []),
])].filter(id => !installationIds.includes(id)),
} } : {}),
repositories: undefined,
@@ -18,6 +18,8 @@ function asRecord(value: unknown): Record<string, unknown> {
return {};
}
type RecordGitHubAttempt = (grant: typeof connectionGrants.$inferSelect) => void;
type GitHubContinuityDependencies = {
now: () => Date;
currentCloudConnector: () => PaperclipCloudConnector | null;
@@ -30,6 +32,7 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
connection: typeof toolConnections.$inferSelect,
initialGrant: typeof connectionGrants.$inferSelect,
actor?: ActorInfo,
recordAttempt?: RecordGitHubAttempt,
) {
await actor?.managementContext?.reauthorize();
return measureRunPhase("connectors.github.refresh", async () => {
@@ -38,6 +41,7 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
connection,
initialGrant,
actor,
recordAttempt,
);
} catch (error) {
if (
@@ -56,7 +60,7 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
),
));
if (!current || current.status !== "active") throw error;
return refreshManagedGitHubGrantAccessOnce(connection, current, actor);
return refreshManagedGitHubGrantAccessOnce(connection, current, actor, recordAttempt);
}
});
}
@@ -65,6 +69,7 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
connection: typeof toolConnections.$inferSelect,
initialGrant: typeof connectionGrants.$inferSelect,
actor?: ActorInfo,
recordAttempt?: RecordGitHubAttempt,
) {
await actor?.managementContext?.reauthorize();
let grant = await measureRunPhase("connectors.github.credentials", () => refreshOAuthGrantCredentials({
@@ -73,6 +78,7 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
grantId: initialGrant.id,
actor,
}));
recordAttempt?.(grant);
const resolveAccessToken = async () => measureRunPhase("connectors.github.credentials", async () => {
const accessRef = grant.credentialSecretRefs.find(
(ref) => ref.configPath === "oauth.access_token",
@@ -113,6 +119,7 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
forceRefresh: true,
actor,
}));
recordAttempt?.(grant);
accessToken = await resolveAccessToken();
try {
metadata = await measureRunPhase("connectors.github.repositories", () => loadGitHubGrantMetadata(
@@ -204,7 +211,7 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
return { updated, previousGitHub };
});
return measureRunPhase("connectors.github.webhooks", () => syncGitHubGrantBindings(connection, updated, actor, accessToken));
return measureRunPhase("connectors.github.webhooks", () => syncGitHubGrantBindings(connection, updated, actor, accessToken, recordAttempt));
}
async function syncGitHubGrantBindings(
@@ -212,6 +219,7 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
initialGrant: typeof connectionGrants.$inferSelect,
actor?: ActorInfo,
accessToken?: string,
recordAttempt?: RecordGitHubAttempt,
) {
await actor?.managementContext?.reauthorize();
const cloudConnector = currentCloudConnector();
@@ -243,9 +251,11 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
const subject = current.kind === "agent" && current.subjectAgentId ? `agent:${current.subjectAgentId}`
: current.kind === "user" ? current.subjectUserId : null;
if (!subject) throw unprocessable("GitHub webhook subject is unavailable");
recordAttempt?.(grant);
const changes = [
...github.installationIds.map(installationId => ({ installationId, active: true as const })),
...github.bindingSync.removeInstallationIds.map(installationId => ({ installationId, active: false as const })),
...github.bindingSync.removeInstallationIds.filter(id => !github.installationIds.includes(id))
.map(installationId => ({ installationId, active: false as const })),
];
for (let offset = 0; offset < changes.length; offset += 4) {
const outcomes = await Promise.allSettled(changes.slice(offset, offset + 4).map(change => cloudConnector.setWebhookBinding({
@@ -265,13 +275,14 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
});
}
async function retainRetry(connection: typeof toolConnections.$inferSelect, grantId: string, error?: unknown, minimumMs = 30_000) {
async function retainRetry(connection: typeof toolConnections.$inferSelect, grantId: string, attemptedRevision: string, error?: unknown, minimumMs = 30_000) {
await db.transaction(async tx => {
const [current] = await tx.select().from(connectionGrants).where(and(
eq(connectionGrants.id, grantId), eq(connectionGrants.companyId, connection.companyId),
eq(connectionGrants.connectionId, connection.id), eq(connectionGrants.status, "active"),
)).for("update").limit(1);
if (!current?.providerTenant?.github) return;
// Backoff belongs to the attempted credential/installation revision.
if (!current?.providerTenant?.github || githubContinuityRevision(current) !== attemptedRevision) return;
const attempts = Math.min(16, (current.providerTenant.github.continuityRetry?.attempts ?? 0) + 1);
const advised = error instanceof PaperclipCloudConnectorError ? error.retryAt
: error instanceof HttpError ? asRecord(error.details).retryAt : undefined;
@@ -280,7 +291,7 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
await notifyGitHubCredentialChange(tx, connection);
await tx.update(connectionGrants).set({
providerTenant: sql`jsonb_set(${connectionGrants.providerTenant}, '{github,continuityRetry}', ${JSON.stringify({
at, attempts, revision: githubContinuityRevision(current),
at, attempts, revision: attemptedRevision,
})}::jsonb)`,
updatedAt: now(),
}).where(and(eq(connectionGrants.id, current.id), eq(connectionGrants.companyId, connection.companyId)));
@@ -324,18 +335,20 @@ export function createGitHubGrantContinuity(db: Db, { now, currentCloudConnector
let refreshed = 0;
let failed = 0;
for (const row of due) {
let attemptedRevision = githubContinuityRevision(row.grant);
const recordAttempt: RecordGitHubAttempt = grant => { attemptedRevision = githubContinuityRevision(grant); };
try {
const actor: ActorInfo = { actorType: "system", actorId: "github-connection-continuity" };
const updated = row.grant.providerTenant?.github?.bindingSync
? await syncGitHubGrantBindings(row.connection, row.grant, actor)
: await refreshManagedGitHubGrantAccess(row.connection, row.grant, actor);
? await syncGitHubGrantBindings(row.connection, row.grant, actor, undefined, recordAttempt)
: await refreshManagedGitHubGrantAccess(row.connection, row.grant, actor, recordAttempt);
// A successful provider response can still leave an unchanged/short-lived
// credential due. Retain that obligation with bounded durable backoff.
const next = githubGrantContinuityDeadline(row.connection, updated);
if (next !== null && next <= now().getTime()) await retainRetry(row.connection, updated.id, undefined, 60 * 60_000);
if (next !== null && next <= now().getTime()) await retainRetry(row.connection, updated.id, githubContinuityRevision(updated), undefined, 60 * 60_000);
refreshed += 1;
} catch (error) {
await retainRetry(row.connection, row.grant.id, error);
await retainRetry(row.connection, row.grant.id, attemptedRevision, error);
failed += 1;
}
}