mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-20 15:09:57 +02:00
refactor(server): isolate provider webhook lifecycle decisions (#16283)
## Thinking Path > - Paperclip helps people manage AI agents for work. > - Chat channels turn external messages into shared task workflows. > - The main service still contains some provider decisions. > - These decisions belong with the existing channel modules. > - This pull request moves webhook recovery and payload-scope decisions to their provider modules. > - Shared authorization, persistence, and runtime ownership keep their current order. ## Linked Issues or Issue Description Refs: #16235. Stack part 2/3. Base: `master`. #16276 is merged. **What existing behavior does this improve?** Provider ownership and readability in the chat channel service. **Current behavior** The entry point combines shared workflows with provider decisions. **Proposed behavior** Provider modules own their decisions. Shared modules own product state and authorization. **Reason and benefit** Maintainers can find provider behavior in its channel folder. **Breaking changes** None intended. Public service methods and durable data shapes stay compatible. ## What Changed - Move GitHub revoked-installation recovery classification to `github/webhook-scope.ts`. - Move Teams payload-scope validation to its message module. - Record normalized GitHub, Telegram, and Teams lifecycle events through one shared durable call. - Keep Slack callback ordering and persistence failures outside parser catches. - Reduce `chat-channels.ts` from 3,598 to 3,570 lines. ## Verification - The same 86 selected webhook/lifecycle integration cases passed before and after in an owned Linux container. - The same 17 selected Telegram edit and Teams edit/delete/restore cases passed before and after in that container. - The same 94 SDK adapter and lifecycle-normalization tests passed locally before and after. - Server typecheck and module-boundary checks passed. - Token comparison confirms the shared recording payload and runtime fence, plus the Teams scope and parser inputs. - The host database baseline failed before assertions because System V shared-memory IDs were exhausted. No other PostgreSQL server or host setting was changed. Database verification used an isolated container under its unprivileged user. - The extraction rebased without conflicts after #16276 merged. Git range-diff reports the same patch. The 94 SDK/lifecycle tests and server typecheck passed on this base. All CI gates pass on rebased head `d835d7c3815c168226fa646aec6370b6a8ac94c1`. Greptile reviewed that exact head at 5/5, with no inline findings or unresolved review threads. - Cumulative `pnpm -r typecheck` and `pnpm build` passed. All 1,321 channel integration cases passed. The normal UI, CLI, database, adapter, plugin, and serialized API test selections ran. - The first Linux `pnpm test:run` returned 1. Original-source runs reproduced the local failures. Container-only repairs supplied missing tools, process reaping, and sufficient Node heap. The existing mocked-fetch CLI telemetry fixture passed with the image opt-out flag overridden only for that fixture. - One local venue limitation remains: adapter-utils T24 fails unchanged on both original and final source in the Linux container. Its entire 56-case suite passes before and after on the host. All other failed or aborted suites passed in repaired reruns. No test assertion or timeout changed. - These fixtures exercise application state and the service entry point. They do not prove live provider execution. ## Risks - A scope or ordering error could record an event for the wrong provider context. - The selected parser returns normalized data. The shared service keeps durable recording and runtime fencing. - No database migration or API change is included. ## Model Used OpenAI GPT-6, running in Codex with reasoning, tool use, and code execution. The exact deployment 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 - [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>
This commit is contained in:
1 parent
74ba530b9e
commit
7df55f3dcc
3 files changed
+29
-40
No files matched your search
@@ -19,7 +19,7 @@ import { createInboundWakeNotices } from "./chat-channels/inbound/wakeup-notices
|
||||
import { createDiscordRuntimeReconciliation } from "./chat-channels/discord/runtime-reconciliation.js";
|
||||
import { createPhotonRuntimeCallbacks } from "./chat-channels/imessage-photon/runtime-callbacks.js";
|
||||
import { createChatReceiverOwnership } from "./chat-channels/runtime/ownership.js";
|
||||
import { createGitHubWebhookScope } from "./chat-channels/github/webhook-scope.js";
|
||||
import { createGitHubWebhookScope, githubWebhookCanRecoverRevokedInstallation } from "./chat-channels/github/webhook-scope.js";
|
||||
import { createGitHubWebhookAuthentication, retryableGitHubWebhookResponse } from "./chat-channels/github/webhook-authentication.js";
|
||||
import { createSlackWebhookCallbacks } from "./chat-channels/slack/webhook-callbacks.js";
|
||||
import { createGitHubWebhookEvents } from "./chat-channels/github/webhook-events.js";
|
||||
@@ -110,7 +110,7 @@ import {
|
||||
import { telegramLifecycleEventFromPayload, telegramIgnoresInboundMessage } from "./chat-channels/telegram/messages.js";
|
||||
|
||||
import {
|
||||
microsoftTeamsLifecycleEventFromPayload, teamsOrphanFollowUp, teamsAttachmentReferenceText,
|
||||
teamsWebhookLifecycleEventFromPayload, teamsOrphanFollowUp, teamsAttachmentReferenceText,
|
||||
} from "./chat-channels/microsoft-teams/messages.js";
|
||||
import {
|
||||
githubSupplementalLifecycleFromRequest,
|
||||
@@ -2888,9 +2888,7 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
)
|
||||
.then((rows) => rows[0] ?? null);
|
||||
const recoveringRevokedGitHubInstallation =
|
||||
endpoint?.status === "revoked" &&
|
||||
provider === "github" &&
|
||||
request.headers.get("x-github-event") === "installation";
|
||||
githubWebhookCanRecoverRevokedInstallation(endpoint?.status, provider, request);
|
||||
if (
|
||||
!endpoint ||
|
||||
endpoint.status === "archived" ||
|
||||
@@ -2985,40 +2983,14 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
if (slackCallbackInspection && response.ok) {
|
||||
await recordSlackWebhookCallback(endpoint, request, slackCallbackInspection, runtimeContext);
|
||||
}
|
||||
if (githubInspection && response.ok) {
|
||||
const lifecycle = await githubSupplementalLifecycleFromRequest(githubInspection);
|
||||
if (lifecycle) {
|
||||
await recordLifecycleDelivery(
|
||||
{
|
||||
endpointId: endpoint.id,
|
||||
...lifecycle,
|
||||
},
|
||||
runtimeContext,
|
||||
);
|
||||
}
|
||||
}
|
||||
if (provider === "telegram" && response.ok && lifecyclePayload) {
|
||||
const lifecycle = telegramLifecycleEventFromPayload(lifecyclePayload);
|
||||
if (lifecycle) {
|
||||
await recordLifecycleDelivery(
|
||||
{
|
||||
endpointId: endpoint.id,
|
||||
...lifecycle,
|
||||
},
|
||||
runtimeContext,
|
||||
);
|
||||
}
|
||||
}
|
||||
if (
|
||||
provider === "microsoft-teams" &&
|
||||
response.ok &&
|
||||
lifecyclePayload &&
|
||||
endpointRuntime.acceptsProviderScope(lifecyclePayload)
|
||||
) {
|
||||
const lifecycle = microsoftTeamsLifecycleEventFromPayload(
|
||||
lifecyclePayload,
|
||||
endpointRuntime,
|
||||
);
|
||||
if (response.ok) {
|
||||
const lifecycle = githubInspection
|
||||
? await githubSupplementalLifecycleFromRequest(githubInspection)
|
||||
: provider === "telegram" && lifecyclePayload
|
||||
? telegramLifecycleEventFromPayload(lifecyclePayload)
|
||||
: provider === "microsoft-teams"
|
||||
? teamsWebhookLifecycleEventFromPayload(lifecyclePayload, endpointRuntime)
|
||||
: null;
|
||||
if (lifecycle) {
|
||||
await recordLifecycleDelivery(
|
||||
{
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import type { LifecycleRuntimeFence } from "../runtime-types.js";
|
||||
import type { EndpointRow, LifecycleRuntimeFence } from "../runtime-types.js";
|
||||
import type { ChatSdkProvider } from "../../chat-sdk-runtime.js";
|
||||
export type GitHubIngressContext = { githubIngressActionId: string; githubIngressFence: LifecycleRuntimeFence; superseded?: boolean };
|
||||
export function createGitHubWebhookScope(provider: ChatSdkProvider, internalContext: GitHubIngressContext | undefined, responseBudgetMs: number | undefined, GITHUB_WEBHOOK_RESPONSE_BUDGET_MS: number) {
|
||||
@@ -58,3 +58,12 @@ export function createGitHubWebhookScope(provider: ChatSdkProvider, internalCont
|
||||
return { replayingDurableGitHubIngress, ignoreSupersededIngress, matchesGitHubIngressFence, githubResponseDeadlineAt, completeGitHubPreflightBeforeDeadline };
|
||||
}
|
||||
export type GitHubWebhookScope = ReturnType<typeof createGitHubWebhookScope>;
|
||||
|
||||
export function githubWebhookCanRecoverRevokedInstallation(
|
||||
status: EndpointRow["status"] | undefined,
|
||||
provider: ChatSdkProvider,
|
||||
request: Pick<Request, "headers">,
|
||||
): boolean {
|
||||
return status === "revoked" && provider === "github" &&
|
||||
request.headers.get("x-github-event") === "installation";
|
||||
}
|
||||
@@ -107,3 +107,11 @@ export function teamsOrphanFollowUp(threadId: string, messageId: string): boolea
|
||||
export function teamsAttachmentReferenceText(count: number, providerUrl: string | null): string {
|
||||
return `Shared ${count} Microsoft Teams file reference${count === 1 ? "" : "s"}.${providerUrl ? ` Open in Microsoft Teams: ${providerUrl}` : ""}`;
|
||||
}
|
||||
|
||||
export function teamsWebhookLifecycleEventFromPayload(
|
||||
payload: unknown,
|
||||
endpointRuntime: Pick<ChatEndpointRuntime, "acceptsProviderScope" | "parseMicrosoftTeamsMessage">,
|
||||
): MicrosoftTeamsLifecycleEvent | null {
|
||||
if (!payload || !endpointRuntime.acceptsProviderScope(payload)) return null;
|
||||
return microsoftTeamsLifecycleEventFromPayload(payload, endpointRuntime);
|
||||
}
|
||||
Reference in new issue
Block a user