Refactor webhook.ts by concern — split HTTP / handlers / CI / config + extract forgejo-api.ts #34
Labels
No labels
area:agents
area:dashboard
area:database
area:design
area:design-review
area:flows
area:infra
area:meta
area:security
area:sessions
area:webhook
area:workdir
security
type:bug
type:chore
type:meta
type:user-story
No milestone
No project
No assignees
1 participant
code-lead
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
charles/claude-hooks#34
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
User story
As a maintainer of claude-hooks, I want
src/webhook.tsdecomposed along its three distinct concerns (HTTP glue, event handlers, CI state machine) plus a reusable Forgejo API client, so that the 1000-line file stops mixing signature verification, event dispatch, and CI dedup timers — and future changes to one concern don't force readers to hold the other two in their head.Context
src/webhook.tshas grown to ~1020 lines. The real weight is the CI state machine (aggregate-state polling, dedup maps, review-fallback timers, fix-ci log prefetch) — not the dispatch switch. A pure per-event split would force every new file to import the same CI helpers, so split by concern, not by event.Acceptance criteria
New file layout
src/webhook.ts— HTTP only. Signature verification, body parse, the event switch, orchestration of handler dispatch + cleanup. Target ≤ 200 lines.src/webhook-handlers.ts— Event handlers. One exported function per event type (handleIssueAssigned,handleReviewRequested,handleChangesRequested,handleApproved,handlePullRequestOpened,handlePullRequestClosed,handleIssueClosed,handleStatusEvent,handleActionRunEvent). No global state; config and Forgejo client injected.src/webhook-ci.ts— CI gate.prMergeable,fetchAggregateState,findOpenPRForHead,fetchFailingRunLogs,dispatchFixCi,requestReviewIfFresh,repoHasWorkflows,armReviewFallback/cancelReviewFallback, the_fixCiDispatchedand_reviewFallbacksstate, theREVIEW_FALLBACK_MSconstant.src/webhook-config.ts— Config.loadWebhookConfig,WebhookConfig,AgentConfigtypes, singleton accessor. (Consider freezing the returned config object.)src/forgejo-api.ts— Centralized HTTP client. Allfetch(\${config.forgejoUrl}/api/v1/...`)calls go through helpers here (getPullRequest,listPullReviews,getAggregateStatus,listWorkflowJobs,getWorkflowLogs,requestReviewers,getContentsList). Shared auth header, shared 5sAbortSignal.timeout`, shared error typing.Behavior-preserving
src/main.tsis unchanged — the HTTP server still imports one symbol fromsrc/webhook.ts(handleForgejoWebhook).src/webhook.test.ts) continue to pass without change. If a test needs to be updated because a handler now lives elsewhere, update the import only; no behavior changes to assert.just qapasses (typecheck + lint + format + tests).charles/dummy-appstill goes issue → dev PR → no-workflows fast path → reviewer → approve → boss merge.Seams enforced
webhook.tsdoes not import anything fromforgejo-api.tsdirectly. Every API call sits behind either a handler (for request processing) orwebhook-ci.ts(for state-machine work).webhook-handlers.tsdoes not mutate global CI state. If a handler needs to arm a fallback or mark a SHA dispatched, it calls awebhook-ci.tsexport.export * from). Every cross-module symbol is named.Tests relocated
webhook.test.ts(routing),webhook-handlers.test.ts(per-handler),webhook-ci.test.ts(state machine). Each file stays under ~300 lines.Out of scope
forgejo-api.tsshould already useAbortSignal.timeout(5000)for the calls it owns, but the in-place patching of remaining timeouts and theBoundedMapintroduction live in #B so #A stays a pure shape refactor.main.tsdecomposition (see #C).References
#B(hardening),#C(main.ts split),#D(tests).Dependencies
#Dmain