refactor(queue): split processGitHubWebhook into per-event handlers - #4695
Merged
Conversation
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4695 +/- ##
==========================================
+ Coverage 94.14% 94.19% +0.05%
==========================================
Files 437 437
Lines 38561 38574 +13
Branches 14058 14061 +3
==========================================
+ Hits 36304 36336 +32
+ Misses 1599 1576 -23
- Partials 658 662 +4
🚀 New features to boost your workflow:
|
loopover-orb
Bot
added
the
gittensor:bug
Contributor
|
Caution 🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥 🛑 Gittensory review result - fixes requiredReview updated: 2026-07-10 13:36:07 UTC
🛑 Suggested Action - Manual Review
Review summary Nits — 6 non-blocking
Why this is blocked
CI checks failing
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Part of #4607 (the processors.ts mega-function breakup). This is the processGitHubWebhook slice of that issue's three-function plan -- maybePublishPrPublicSurface is a separate, much larger follow-up, and runAgentMaintenancePlanAndExecute's own slice ships as its own PR. The issue's own finding for this function: it "inlines the reopen-reclose/draft-dodge/review-evasion trigger conditions directly alongside dozens of other event/action branches." This PR extracts eight of those branches into named, single-purpose handler functions, leaving the router as a thin dispatcher over the same event-type checks, in the same order: - maybeHandleInstallationDeletedWebhookEvent - maybeHandleForeignAppInstallationWebhookEvent - handleInstallationRepositoriesWebhookEvent - handleInstallationCreatedWebhookEvent - maybeHandleReactionWebhookEvent - maybeHandleIssueCommentCommandWebhookEvent (the 14-branch issue_comment command/mention dispatch chain) - handlePullRequestWebhookEvent (the pull_request-carrying payload block: outcome/reversal recording, cache invalidation, reopen- reclose, draft-dodge, review-evasion, the gate + auto-maintain pipeline, reputation, RAG/sibling-regate enqueueing) - handleIssueWebhookEvent (the non-PR issue block) Pure code motion, zero behavior change. Every extracted block was cut from its exact original location and dedented programmatically (no manual retyping), so a scripted round-trip -- reversing the dedent and the "exit early" boolean-return signal back to the original bare `return` -- reproduces the original inline text byte-for-byte. The five non-PR wake triggers already implemented as separate `maybeX` helper functions elsewhere in the file, and the notification-event detection/enqueue epilogue, are left inline in the router unchanged -- they either already match the target "named handler, thin call site" shape or are cross-cutting bookkeeping rather than a per-event-type branch. processGitHubWebhook itself shrinks from 947 to 153 lines. Validated: typecheck clean; test/unit/queue.test.ts's full 807 tests pass unmodified; test/unit/actions-fallback-webhook.test.ts's 17 tests (also exercises this file's job processing) pass unmodified. Scoped coverage (queue.test.ts plus every other test file that references processGitHubWebhook) shows the diff's added lines fully exercised, with one pre-existing gap (a deployment_status early-return with no dedicated test) confirmed present at its original line number before this change too, via the same test set against unmodified main.
…extraction PR #4695 extracted 8 named handler functions from processGitHubWebhook, a pure code-motion refactor -- but relocating previously-untested lines still counts them as "new" against the patch-coverage diff, and codecov/patch correctly failed at 97.89% (target 99%). Two of the gaps are genuinely reachable and now have real regression tests: - handleInstallationCreatedWebhookEvent's targetKey ternary fallback: unlike its installation_repositories sibling, this handler's guard does not require installation.id, so a malformed/partial delivery genuinely reaches the repoFullName fallback arm. - maybeCaptureOnDeploymentStatus's early-return had zero coverage anywhere in the whole suite, not just this diff. The remaining gaps are marked /* v8 ignore next */ with a one-line reason, matching this file's own established defensive/best-effort convention (mirroring the existing comment style already used a few lines away): - Both installation_repositories ternary fallback arms are mathematically unreachable -- the enclosing guard already requires the same payload.installation?.id truthy check to reach that block. - The three terminalizeActiveReviewTracking/invalidatePrStateCache best-effort .catch(() => undefined) sites were confirmed pre-existing (zero coverage on origin/main before this PR existed too). Part of #4607
JSONbored
force-pushed
the
refactor/processors-webhook-router-4607
branch
from
July 10, 2026 20:17
073f477 to
8420659
Compare
This was referenced Jul 10, 2026
19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
processors.tsmega-function breakup). This PR does theprocessGitHubWebhookslice of that issue —
maybePublishPrPublicSurfaceis a separate, much larger follow-up, andrunAgentMaintenancePlanAndExecute's own slice ships as its own PR.trigger conditions directly alongside dozens of other event/action branches." This PR extracts eight
of those branches into named, single-purpose handler functions (colocated in
src/queue/processors.ts,immediately above
processGitHubWebhook, in the same order they're called), leaving the router as athin dispatcher over the same checks in the same order:
maybeHandleInstallationDeletedWebhookEventmaybeHandleForeignAppInstallationWebhookEventhandleInstallationRepositoriesWebhookEventhandleInstallationCreatedWebhookEventmaybeHandleReactionWebhookEventmaybeHandleIssueCommentCommandWebhookEvent(the 14-branchissue_commentcommand/mention dispatchchain — panel retrigger, panel generate-tests, gate-override, resolve/explain/generate-tests/review/
pause/resume/configuration/plan mention commands, review-nag cooldown throttling, monitored-mention
throttling, and the general
@gittensorymention command)handlePullRequestWebhookEvent(thepull_request-carrying payload block: outcome/reversal signalrecording, reviews-cache and mergeable-state cache invalidation, the one-shot reopen-reclose guard,
draft-dodge / review-evasion enforcement, the readiness → gate → auto-maintain pipeline, reputation
recording, RAG re-index and sibling-regate enqueueing)
handleIssueWebhookEvent(the non-PRissueblock: issue advisory, slop triage, account-agelabeling, the per-contributor open-issue cap, and issue-watch: multiplier-aware new-issue monitoring for miners (gittensory_watch_issues) #699 path B issue-watch detection)
maybeReReviewOnCiCompletion,maybeCaptureOnActionsFallbackWorkflowRun,maybeInvalidateCiCacheOnLegacyCiEvent,maybeCaptureOnDeploymentStatus,maybeReReviewOnLinkedIssueChange) were already separately-definedmaybeX(...)-returning-boolean helper functions called from the router — they already match thetarget "named handler, thin call site" shape, so there was nothing to extract. The notification-event
detection/chunked-enqueue epilogue and the
installation/repositoriesupsert bookkeeping arecross-cutting (they run regardless of event type, not gated on one), not per-event-type branches, so
they stay inline in the router rather than being force-fit into an "event handler."
location and mechanically dedented (no manual retyping of the moved logic, to eliminate transcription
risk on a 15k-line live-webhook file). A scripted round-trip verification — reversing the dedent and
reversing the "exit early" boolean-return signal back to the original bare
return— reproduces theoriginal inline text byte-for-byte for all eight extracted functions. Two structural checks confirm
the router itself preserves order/short-circuiting exactly: (1) the one nested
return;that used toexit
processGitHubWebhookdirectly from inside the reopen-reclose branch is nowreturn true;, withthe router doing
if (await handlePullRequestWebhookEvent(...)) return;immediately after — so theearly-exit still skips issue-handling/notification-enqueue/the final
recordWebhookEventexactly asbefore; (2) the file outside the touched function (everything before line 5736 and after the function's
old closing brace) is untouched, confirmed identical line-for-line against the pre-change file.
processGitHubWebhookshrinks from 947 to 153 lines (an 83.9% reduction), now a genuinelythin dispatcher.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Note on the issue link: this is
Part of #4607, notCloses/Fixes— the issue explicitly covers twomore functions (
maybePublishPrPublicSurface, andrunAgentMaintenancePlanAndExecute, the latter mid-flightas its own PR) as separate PRs. See the issue body: "this is expected to land as multiple sequential PRs
(one per function)."
Validation
git diff --checknpm run actionlint— not run; no.github/workflows/**files touched.npm run typecheck— clean, both before rebasing and re-confirmed against freshorigin/mainimmediately before push (no new commits landed on
mainin between, so no rebase was actuallyneeded —
origin/mainwas still at the exact commit this branch was created from).npm run test:coverage— not run as the literal full-suite command; a scoped coverage run (below)proved the diff itself is fully exercised, matching the precedent set by the sibling
runAgentMaintenancePlanAndExecuteextraction PR for this same issue.npm run test:workers— not run; notest/workers/**-relevant code touched.npm run build:mcp/npm run test:mcp-pack— not run; no MCP package changes.npm run ui:openapi:check/npm run ui:lint/npm run ui:typecheck/npm run ui:build— notrun; no
apps/gittensory-ui/**or API/schema changes.npm audit --audit-level=moderate— not run; no dependency changes.If any required check was skipped, explain why:
src/queue/processors.tswith no UI, MCP,workers, schema, OpenAPI, workflow, or dependency surface touched, so those gates are left to CI rather
than duplicated locally (most path-filter out for this diff anyway). The checks that matter for a pure
refactor — typecheck and the exercising test suite — were run directly, in full, in the foreground:
npm run typecheck: clean.npx vitest run test/unit/queue.test.ts: 807/807 tests passed, unmodified — the primary suiteexercising
processGitHubWebhookend-to-end (webhook dispatch, reopen-reclose, draft-dodge,review-evasion, type-label decoupling, auto-action convergence, agent re-gate sweep). No existing
assertion was touched.
npx vitest run test/unit/actions-fallback-webhook.test.ts: 17/17 tests passed, unmodified —also imports
processJobfrom this same source file.queue.test.ts; none exists —webhook.test.ts/github-webhook-coalesce.test.ts/orb-webhook.test.tstest the signature-verification/ingestion/coalesce-key layers, a different part of the pipeline from the queue-side
router this PR touches.
npx vitest run test/unit/queue.test.ts --coverage, cross-referenced against the exactadded-line ranges from the diff. Of 165 added, executable lines, 162 were covered by
queue.test.tsalone. Extending the run to every other test file that references
processGitHubWebhook(
actions-fallback-webhook.test.ts,reputation-wiring.test.ts,safety.test.ts,parity-wire.test.ts,unified-comment-bridge.test.ts— 958 tests total, all passing) covers 2 ofthe remaining 3. The last uncovered line (a
deployment_statusearly-return with no dedicated test)was confirmed present and already uncovered at its original line number, with the identical
6-file test set, against unmodified
main— a pre-existing gap this refactor merely relocates,not one it introduces.
Safety
UI Evidencesection below... — N/A, no visible/UI changes (backend-only refactor).CHANGELOG.mdintentionally not touched.UI Evidence
Not applicable — this PR has no visible/UI/frontend/docs surface; it is a backend-only, behavior-preserving
extraction inside
src/queue/processors.ts.Notes
maintainer-only).suite: every extracted function was round-tripped programmatically (dedent reversed, boolean-return
signal reversed back to the original bare
return) and diffed against the original inline text —byte-identical for all eight. The file outside the touched function is confirmed line-for-line identical
to before this change.
runAgentMaintenancePlanAndExecute's plan-input extraction(in flight), and
maybePublishPrPublicSurface's step extraction (not started — the largest of thethree, will need its own sequence of PRs given its size).