mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-01 06:42:06 +01:00
fix(server): wake GitHub acknowledgements from committed changes (#17656)
<!-- entire-trail-link-start --> https://entire.com/gh/paperclipai/-/trails/206/changes/paperclip/206 <!-- entire-trail-link-end --> ## Thinking Path > - Paperclip coordinates agent work and presents its progress in chat. > - GitHub deliveries save acknowledgement actions and provider receipts. > - Periodic maintenance currently searches for unfinished acknowledgements even when idle. > - Committed delivery, lease, response and presentation changes identify when work can advance. > - The existing owner should wake on those changes and recover only at saved deadlines. > - Completed history should become quiet without losing unfinished provider effects. ## Linked Issues or Issue Description Refs #17653, which is merged. This existing GitHub acknowledgement slice targets master. **Problem:** A saved GitHub acknowledgement can wait for a publication lease, fast-response result or native presentation decision. Polling empty or unchanged work keeps the tenant active. Removing maintenance without covering these producers can strand the acknowledgement. **Expected behavior:** Committed changes wake the scoped owner. It reconstructs existing actions on startup, retries at saved deadlines, parks unchanged presentation waits and becomes quiet when settled. ## What Changed - Register GitHub acknowledgement work with the shared scoped deadline coordinator. Reuse existing action, delivery, lease and fast-response rows. - Add transaction-aware response-availability notifications at supported writes. Keep GitHub protocol details in its platform owner and shared notifications limited to IDs. - Share the bounded latest-request/run projection between scheduling and direct response authorization. An absent native presentation decision parks the action until its real writer commits. - Retain publication leases, source/principal authority, version fencing, response markers and final receipt precedence. Check admission before each new provider request; let in-flight work settle. Stop does not consume the provider retry budget. - Remove only recurring acknowledgement recovery and keep its manual path. Add no work table or scheduler service. - Regenerate the three original indexes as `0400_absent_phantom_reporter.sql`: pending acknowledgement IDs, fast-response delivery lookup and publication comment/destination lookup. Each uses concurrent/idempotent creation and the existing top-level migration runner. Preserve all landed SQL/journal entries and the five newest snapshots. ## Verification Published head: `09ca00b9f029838434ec042cda691345d67b493e`, on landed continuation parent `063cfd43813cabf18417605fb9eefb9aacf95c8d`. - **87 distinct current cases pass:** 18 real PostgreSQL acknowledgement lifecycle/presentation cases, all 26 fast-response service cases, all 30 heartbeat completion cases and all 13 startup cases. Database build, server typechecking/build, module boundaries and three fresh migrations through 0400 pass. The current Runner bundle rebuild also passes after upstream provider-event changes. - The lifecycle matrix covers empty/history quiet, lease release, restart, drain/standby, fast-response failure/expiry/publication/unknown outcome, streaming quarantine, outer rollback/commit, saved provider retry, stop between requests and receipt state. The real presentation writer proves quiet while pending, rollback, committed reactivation and final quiet. - The earlier **305-case** matrix (236 full GitHub workflow cases, 26 fast-response service cases, 30 completion cases and 13 startup cases) passes at `337f272853d4f8a51b5b9300900a46eb4e13f698`. It retains its prepared-parent scope. Repeated cases are not added to current 87. - A populated 0399 database upgrades to 0400 with all 398 prior migration receipts unchanged, one new receipt and no additional receipt on replay. All three indexes are valid/ready on fresh and upgraded databases. Their SQL definitions match the original draft after whitespace normalization; migration history and retained snapshot bytes are preserved. - Normal integration `9b3607e159153e00ccdfe538d52b591bd38d6499` preserves the full 28-path owned binary patch and all 20 unrelated master paths, including native retry, provider-event and evaluation workflow changes. Both normal merge parents and the exact tree are checked. No force push or rebase occurs. - [The owner audit](https://github.com/paperclipai/paperclip/blob/09ca00b9f029838434ec042cda691345d67b493e/doc/plans/2026-10-25-github-chat-maintenance-sleep-audit.md#acceptance-on-the-actual-landed-continuation-parent) retains the original missing-notification, JSON precedence, receipt lifecycle, external-database fixture and migration-registry failures. Earlier 69/228/broad counts keep their original scope. Skips and failed revisions are not relabelled as current passes. - The final audit-only edit leaves all owned source/test/migration hashes unchanged. **Fresh exact-head CI and Greptile 5/5 with no actionable findings remain required.** No full local repository, live GitHub tenant or deployed sleep result is claimed. ## Risks - Process-local transaction hints cover supported application writers and shared pools. Independent SQL writers and other replicas do not gain a new wake guarantee. - Provider effects remain guarded by the existing publication lease and current source/principal authority. Unknown outcomes retain their existing recovery semantics. - Presentation waits depend on their supported milestone/transaction producers. A missing producer can defer recovery until another supported change or restart. - Index creation can take time on large histories and runs concurrently outside a transaction. Existing migration recovery handles replay; prior SQL is unchanged. - Worker quiet is distinct from host/database suspension. Cloud wake coverage and sleep admission remain unchanged; later maintenance duties are separate slices. ## Model Used OpenAI Codex, using reasoning, repository inspection, code execution and real PostgreSQL tests. The exact model identifier 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 - [ ] All Paperclip CI gates are green - [ ] 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>
This commit is contained in:
DottaandPaperclip authored and GitHub committed
1 parent
063cfd4381
commit
ba5bba86dc
28 files changed
+1253
-111
No files matched your search
@@ -1,3 +1,4 @@
|
||||
import { readGitHubResponseRun, responseRunColumns } from "./chat-channels/github/response-run-state.js";
|
||||
import { and, asc, desc, eq, inArray, or, sql } from "drizzle-orm";
|
||||
import {
|
||||
chatActions, chatConversations, chatDeliveries, chatEndpoints,
|
||||
@@ -5,6 +6,9 @@ import {
|
||||
fastResponseRequests, chatPublications,
|
||||
type Db,
|
||||
} from "@paperclipai/db";
|
||||
import { notifyChatResponseAvailability } from "./chat-response-availability.js";
|
||||
import { registerGitHubAcknowledgementWork } from "./chat-channels/github/acknowledgement-work.js";
|
||||
import type { createDeliveryWorkCoordinator } from "./delivery-work-coordinator.js";
|
||||
import { conflict, forbidden, HttpError } from "../errors.js";
|
||||
import { githubChatPrincipalAccess } from "./chat-github-access.js";
|
||||
import { githubAutomaticAdmission } from "./chat-github-events.js";
|
||||
@@ -25,51 +29,8 @@ type Api = { request<T>(path: string, options?: { method?: string; body?: unknow
|
||||
type Lease = { commit<T>(write: (tx: Transaction) => Promise<T>): Promise<T> };
|
||||
type Source = GitHubResponseSource;
|
||||
type Comment = { id: number; body?: string; html_url: string; user?: { login?: string } };
|
||||
const responseRunColumns = {
|
||||
id: heartbeatRuns.id,
|
||||
companyId: heartbeatRuns.companyId,
|
||||
agentId: heartbeatRuns.agentId,
|
||||
responsibleUserId: heartbeatRuns.responsibleUserId,
|
||||
activeIdentityContextId: heartbeatRuns.activeIdentityContextId,
|
||||
nativeIssueId: heartbeatRuns.nativeIssueId,
|
||||
issueId: heartbeatRuns.issueId,
|
||||
status: heartbeatRuns.status,
|
||||
updatedAt: heartbeatRuns.updatedAt,
|
||||
// Terminal execution can precede native presentation commit. Until its
|
||||
// durable decision exists, missing output is undecided rather than failure.
|
||||
nativePresentationPending: sql<boolean>`coalesce(
|
||||
${heartbeatRuns.status} = 'succeeded'
|
||||
and ${heartbeatRuns.runtimeMode} = 'native'
|
||||
and (${heartbeatRuns.nativePhase} is distinct from 'committed'
|
||||
or ${heartbeatRuns.resultJson}->'presentationDecision'->>'schema' is distinct from 'paperclip.run_presentation_decision.v1'
|
||||
or coalesce(${heartbeatRuns.resultJson}->'presentationDecision'->>'commentAction', '') not in ('none', 'create', 'reuse')),
|
||||
false
|
||||
)`,
|
||||
// Project only the server-committed no-publication decision, never the
|
||||
// potentially large native result or model-authored summary.
|
||||
passiveResponseWait: sql<boolean>`coalesce(
|
||||
${heartbeatRuns.status} = 'succeeded'
|
||||
and ${heartbeatRuns.runtimeMode} = 'native'
|
||||
and ${heartbeatRuns.nativePhase} = 'committed'
|
||||
and ${heartbeatRuns.resultJson}->>'finalizationPhase' = 'committed'
|
||||
and ${heartbeatRuns.resultJson}->>'finalizationReasonCode' = 'external_chat_response_waiting'
|
||||
and ${heartbeatRuns.resultJson}->'nativeResult'->>'schema' = 'paperclip.run_result.v1'
|
||||
and ${heartbeatRuns.resultJson}->'nativeResult'->>'reportedWorkDisposition' = 'yielded'
|
||||
and ${heartbeatRuns.resultJson}->'nativeResult'->'continuation'->>'kind' = 'response_wake'
|
||||
and ${heartbeatRuns.resultJson}->'presentationDecision'->>'schema' = 'paperclip.run_presentation_decision.v1'
|
||||
and ${heartbeatRuns.resultJson}->'presentationDecision'->>'chosenSource' = 'none'
|
||||
and ${heartbeatRuns.resultJson}->'presentationDecision'->>'commentAction' = 'none'
|
||||
and ${heartbeatRuns.resultJson}->'presentationDecision'->'reasonCodes' @> '["yielded_control_plane_wait"]'::jsonb,
|
||||
false
|
||||
)`,
|
||||
contextSnapshot: sql<Record<string, unknown>>`jsonb_build_object(
|
||||
'issueId', ${heartbeatRuns.contextSnapshot}->'issueId',
|
||||
'taskId', ${heartbeatRuns.contextSnapshot}->'taskId',
|
||||
'wakeCommentId', ${heartbeatRuns.contextSnapshot}->'wakeCommentId',
|
||||
'wakeCommentIds', ${heartbeatRuns.contextSnapshot}->'wakeCommentIds'
|
||||
)`,
|
||||
};
|
||||
const generation = (endpoint: Source["endpoint"]) => (endpoint.setup as { runtimeGeneration?: number }).runtimeGeneration ?? 0;
|
||||
const stoppedAcknowledgement = Symbol("stopped GitHub acknowledgement");
|
||||
const actionKey = (deliveryId: string) => `github_response:${deliveryId}`;
|
||||
const responseMarker = (source: Source) => isMainPullResponse(source) ? pullResponseMarker(source) : requestResponseMarker(source);
|
||||
|
||||
@@ -91,18 +52,23 @@ async function findResponseComment(source: Source, api: Api, legacyMarkers: stri
|
||||
* Reconnect epochs fence publication attempts, not the durable comment identity.
|
||||
* Legacy receipts are also checked against the App-owned provider comment. */
|
||||
async function stageResponse(db: Db, source: Source) {
|
||||
const [inserted] = await db.insert(chatActions).values({
|
||||
companyId: source.endpoint.companyId, endpointId: source.endpoint.id,
|
||||
conversationId: source.conversation.id, deliveryId: source.delivery.id,
|
||||
principalId: source.delivery.principalId, kind: "github_response_comment",
|
||||
providerActionId: actionKey(source.delivery.id),
|
||||
payload: { botExternalId: source.endpoint.botExternalId, agentId: source.endpoint.assignedAgentId },
|
||||
status: "received",
|
||||
}).onConflictDoNothing().returning();
|
||||
const response = inserted ?? (await db.select().from(chatActions).where(and(
|
||||
const lookup = () => db.select().from(chatActions).where(and(
|
||||
eq(chatActions.companyId, source.endpoint.companyId), eq(chatActions.endpointId, source.endpoint.id),
|
||||
eq(chatActions.providerActionId, actionKey(source.delivery.id)),
|
||||
)))[0];
|
||||
)).then(rows => rows[0]);
|
||||
const existing = await lookup();
|
||||
const response = existing ?? await db.transaction(async tx => {
|
||||
await notifyChatResponseAvailability(tx, { companyId: source.endpoint.companyId, deliveryId: source.delivery.id });
|
||||
const [inserted] = await tx.insert(chatActions).values({
|
||||
companyId: source.endpoint.companyId, endpointId: source.endpoint.id,
|
||||
conversationId: source.conversation.id, deliveryId: source.delivery.id,
|
||||
principalId: source.delivery.principalId, kind: "github_response_comment",
|
||||
providerActionId: actionKey(source.delivery.id),
|
||||
payload: { botExternalId: source.endpoint.botExternalId, agentId: source.endpoint.assignedAgentId },
|
||||
status: "received",
|
||||
}).onConflictDoNothing().returning();
|
||||
return inserted;
|
||||
}) ?? await lookup();
|
||||
if (!response || response.conversationId !== source.conversation.id ||
|
||||
(response.payload.botExternalId !== undefined && response.payload.botExternalId !== source.endpoint.botExternalId) ||
|
||||
response.payload.agentId !== source.endpoint.assignedAgentId)
|
||||
@@ -135,6 +101,10 @@ export async function writeGitHubResponseComment(
|
||||
(input.versionAt && Date.parse(String(response.result.versionAt)) > input.versionAt.getTime()))) {
|
||||
if (!input.acknowledgement && response.result.final === true && !input.final)
|
||||
throw conflict("This GitHub response is already final");
|
||||
if (["received", "failed"].includes(response.status)) await lease.commit(async tx => {
|
||||
await tx.update(chatActions).set({ status: "processed", updatedAt: new Date() })
|
||||
.where(and(eq(chatActions.id, response.id), eq(chatActions.status, response.status)));
|
||||
});
|
||||
return { id: String(response.result.id), url: String(response.result.url) };
|
||||
}
|
||||
const marker = responseMarker(source);
|
||||
@@ -213,13 +183,18 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
throw forbidden("Invalid GitHub request repository");
|
||||
return { ...row, repositoryId, repository: match[1]!, number: Number(match[2]), replyId: match[3] ?? null };
|
||||
}
|
||||
async function acknowledge(deliveryId: string, finalBody?: string, assertRun?: () => Promise<void>) {
|
||||
async function acknowledge(deliveryId: string, finalBody?: string, assertRun?: () => Promise<void>, canContinue = () => true) {
|
||||
if (!canContinue()) return;
|
||||
const source = await context(deliveryId);
|
||||
// Persist before competing for the publication lane. A busy lane leaves
|
||||
// durable acknowledgement work for maintenance instead of losing it.
|
||||
await stageResponse(db, source);
|
||||
if (!canContinue()) return;
|
||||
return withGitHubPublicationLease(db, { companyId: source.endpoint.companyId, endpointId: source.endpoint.id,
|
||||
repositoryId: source.repositoryId, number: source.number }, fetchImpl, async lease => {
|
||||
repositoryId: source.repositoryId, number: source.number }, (input, init) => {
|
||||
if (!canContinue()) throw stoppedAcknowledgement;
|
||||
return fetchImpl(input, init);
|
||||
}, async lease => {
|
||||
const assertCurrent = async () => {
|
||||
await assertRun?.();
|
||||
const current = await context(deliveryId);
|
||||
@@ -234,22 +209,11 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
let preserveFinal = false;
|
||||
if (finalBody === undefined) {
|
||||
const existing = await stageResponse(db, source);
|
||||
const [requestRun] = await db.select({ runId: heartbeatRuns.id }).from(heartbeatRuns).innerJoin(chatMessageLinks, and(
|
||||
eq(chatMessageLinks.companyId, source.endpoint.companyId), eq(chatMessageLinks.endpointId, source.endpoint.id),
|
||||
eq(chatMessageLinks.deliveryId, deliveryId), eq(chatMessageLinks.direction, "inbound"),
|
||||
or(sql`${chatMessageLinks.commentId}::text = ${heartbeatRuns.contextSnapshot}->>'wakeCommentId'`,
|
||||
sql`coalesce(${heartbeatRuns.contextSnapshot}->'wakeCommentIds', '[]'::jsonb) ? ${chatMessageLinks.commentId}::text`),
|
||||
)).where(and(eq(heartbeatRuns.companyId, source.endpoint.companyId), eq(heartbeatRuns.agentId, source.endpoint.assignedAgentId),
|
||||
sql`coalesce(${heartbeatRuns.nativeIssueId}::text, ${heartbeatRuns.contextSnapshot}->>'issueId') = ${source.conversation.issueId}`))
|
||||
.orderBy(desc(heartbeatRuns.createdAt)).limit(1);
|
||||
const [latestRun] = await db.select(responseRunColumns).from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.companyId, source.endpoint.companyId), eq(heartbeatRuns.agentId, source.endpoint.assignedAgentId),
|
||||
sql`coalesce(${heartbeatRuns.nativeIssueId}::text, ${heartbeatRuns.contextSnapshot}->>'issueId') = ${source.conversation.issueId}`,
|
||||
)).orderBy(desc(heartbeatRuns.createdAt), desc(heartbeatRuns.id)).limit(1);
|
||||
const originMessageId = latestRun?.activeIdentityContextId ? await chatRunSourceMessageId(db, latestRun,
|
||||
{ allowLegacyWakeComment: true, provider: "github" }) : null;
|
||||
const ownsLatestRequest = !!latestRun && (requestRun?.runId === latestRun.id ||
|
||||
(!!source.sourceMessageId && originMessageId === source.sourceMessageId));
|
||||
const { latestRun, ownsLatestRequest } = await readGitHubResponseRun(db, {
|
||||
companyId: source.endpoint.companyId, endpointId: source.endpoint.id,
|
||||
agentId: source.endpoint.assignedAgentId, issueId: source.conversation.issueId,
|
||||
deliveryId, sourceMessageId: source.sourceMessageId,
|
||||
});
|
||||
requestRunning = ownsLatestRequest && latestRun?.status === "running";
|
||||
const terminal = latestRun && ["succeeded", "failed", "interrupted", "timed_out", "cancelled"].includes(latestRun.status);
|
||||
const run = terminal && (ownsLatestRequest || source.issueStatus === "blocked") ? latestRun : null;
|
||||
@@ -289,7 +253,10 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
(await stagePullResponse(db, source)).result?.id === existing.result.id)) {
|
||||
// Rotate no-op maintenance checks so a page of newer queued asks
|
||||
// cannot hide other conversations that need terminal reconciliation.
|
||||
await lease.commit(async tx => { await tx.update(chatActions).set({ updatedAt: new Date() }).where(eq(chatActions.id, existing.id)); });
|
||||
await lease.commit(async tx => { await tx.update(chatActions).set({
|
||||
status: ["received", "failed"].includes(existing.status) ? "processed" : existing.status,
|
||||
updatedAt: new Date(),
|
||||
}).where(and(eq(chatActions.id, existing.id), eq(chatActions.status, existing.status))); });
|
||||
return { id: String(existing.result.id), url: String(existing.result.url) };
|
||||
}
|
||||
if (finalBody === undefined) {
|
||||
@@ -439,6 +406,9 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
or(eq(chatActions.status, "received"), and(eq(chatActions.status, "failed"), sql`${chatActions.result}->>'retryable' = 'true'`,
|
||||
sql`(${chatActions.result}->>'retryAt')::timestamptz <= now()`)))).orderBy(asc(chatActions.createdAt)).limit(limit);
|
||||
for (const row of rows) if (row.deliveryId) await tryAcknowledge(row.deliveryId);
|
||||
return rows.length + await processTerminalResponses(limit);
|
||||
}
|
||||
async function processTerminalResponses(limit = 10) {
|
||||
// Recovery can stop execution before a milestone publication is staged.
|
||||
// Revisit acknowledged requests with terminal execution, even after a
|
||||
// restart; successful acknowledgement is not successful task completion.
|
||||
@@ -463,21 +433,28 @@ export function githubResponseCommentService(db: Db, fetchImpl = fetch) {
|
||||
or coalesce(r.context_snapshot->'wakeCommentIds', '[]'::jsonb) ? l.comment_id::text))))`,
|
||||
)).orderBy(asc(chatActions.updatedAt)).limit(limit);
|
||||
for (const row of stopped) if (row.deliveryId) await tryAcknowledge(row.deliveryId);
|
||||
return rows.length + stopped.length;
|
||||
return stopped.length;
|
||||
}
|
||||
async function tryAcknowledge(deliveryId: string) {
|
||||
try { return await acknowledge(deliveryId); }
|
||||
async function tryAcknowledge(deliveryId: string, canContinue = () => true) {
|
||||
try { return await acknowledge(deliveryId, undefined, undefined, canContinue); }
|
||||
catch (error) {
|
||||
if (error === stoppedAcknowledgement || !canContinue()) return;
|
||||
const [row] = await db.select().from(chatActions).where(and(eq(chatActions.kind, "github_response_comment"), eq(chatActions.deliveryId, deliveryId)));
|
||||
if (!row || row.result?.final === true) return;
|
||||
const attempts = Number(row.result?.attempts ?? 0) + 1;
|
||||
const denied = error instanceof Error && "status" in error && error.status === 403;
|
||||
await db.update(chatActions).set({ status: denied ? "cancelled" : "failed", result: {
|
||||
...row.result, attempts, retryable: !denied && attempts < 8,
|
||||
retryAt: new Date(Date.now() + Math.min(300000, 1000 * 2 ** attempts)).toISOString(),
|
||||
code: denied ? "authorization_changed" : "acknowledgement_unconfirmed",
|
||||
}, updatedAt: new Date() }).where(and(eq(chatActions.id, row.id), sql`coalesce(${chatActions.result}->>'final', 'false') <> 'true'`));
|
||||
await db.transaction(async tx => {
|
||||
await notifyChatResponseAvailability(tx, { companyId: row.companyId, deliveryId });
|
||||
await tx.update(chatActions).set({ status: denied ? "cancelled" : "failed", result: {
|
||||
...row.result, attempts, retryable: !denied && attempts < 8,
|
||||
retryAt: new Date(Date.now() + Math.min(300000, 1000 * 2 ** attempts)).toISOString(),
|
||||
code: denied ? "authorization_changed" : "acknowledgement_unconfirmed",
|
||||
}, updatedAt: new Date() }).where(and(eq(chatActions.id, row.id), sql`coalesce(${chatActions.result}->>'final', 'false') <> 'true'`));
|
||||
});
|
||||
}
|
||||
}
|
||||
return { tryAcknowledge, processPending, failRun, finishRun, hasWorkingResponseForRun };
|
||||
return { tryAcknowledge, processPending, processTerminalResponses, failRun, finishRun, hasWorkingResponseForRun,
|
||||
registerAcknowledgementWork: (coordinator: ReturnType<typeof createDeliveryWorkCoordinator>, canRun: () => boolean) =>
|
||||
registerGitHubAcknowledgementWork(db, coordinator, tryAcknowledge, canRun),
|
||||
};
|
||||
}
|
||||
Reference in new issue
Block a user