charles/claude-hooks

Fork 0

refactor(workflow): split event-handlers.ts into topical modules; delete legacy* aliases #1101

Closed

claude-desktop wants to merge 1 commit from refactor/event-handlers-split into main

claude-desktop commented

2026-05-11 02:29:12 +00:00

Collaborator

Summary

Closes #1096.
Closes #1095.

The 1100-LOC event-handlers.ts god-module is split into five focused files matching the actual concern boundaries. The three legacy* aliases in main.ts (Phase 4 / #1095) are dropped during the move — their function targets now live in workflow/pr-flow.ts with their plain names, so the prFlow capability impls import directly.

New modules

File	Exports
`workflow/review-detection.ts`	`fetchPrDispatchLabels`, `latestVerdict`, `detectOutstandingChangeRequest` (+ private `parseClosesIssue`, `fetchLinkedIssueLabels`)
`workflow/dispatch.ts`	`dispatchIssueForAgent`, `dispatchPrRebase`, `handlePostMergeRebase`, `DispatchPrRebaseStatus`, the post-merge-rebase dedup map (`alreadyDispatchedPostMergeRebase` / `markPostMergeRebaseDispatched` / `resetPostMergeRebaseDedup`)
`workflow/trust.ts`	`isTrustedUser`
`workflow/pr-flow.ts`	`handleIssueClosed`, `handlePrDependencyMarkers`, `handleStackedRebaseCascade`, the trigger-bus slot (`setTriggerBus` / `resetTriggerBus` / private `_triggerBus`), private `agentsForCleanup`
`workflow/issue-unassign.ts`	`handleIssueUnassigned`

Moved into existing `slash-commands.ts`

ApplyReadyStackCommandOpts interface + applyReadyStackCommand impl

Deleted

apps/server/src/domain/workflow/event-handlers.ts (0 LOC after the moves)
apps/server/src/domain/workflow/event-handlers.test.ts (tests redistributed alongside their functions)
The legacyHandle* aliases in main.ts:121-123 — replaced by plain-named imports

Updated callers

~15 files across background/, domain/dispatch/, domain/flows-yaml/, domain/views/, domain/workflow/, http/, main.ts.

Test plan

Pure refactor — zero behavioural change
bun run typecheck clean
Full server suite: 3271 pass / 0 fail (same count as before — refactor, not feature)
bun run biome check clean (the single warning is in a pre-existing apps/server/src/infrastructure/agent-env-sync/render-for-instance.ts stat import, untouched here)
CI green on this PR

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

## Summary Closes #1096. Closes #1095. The 1100-LOC `event-handlers.ts` god-module is split into five focused files matching the actual concern boundaries. The three `legacy*` aliases in `main.ts` (Phase 4 / #1095) are dropped during the move — their function targets now live in `workflow/pr-flow.ts` with their plain names, so the `prFlow` capability impls import directly. ### New modules | File | Exports | |---|---| | `workflow/review-detection.ts` | `fetchPrDispatchLabels`, `latestVerdict`, `detectOutstandingChangeRequest` (+ private `parseClosesIssue`, `fetchLinkedIssueLabels`) | | `workflow/dispatch.ts` | `dispatchIssueForAgent`, `dispatchPrRebase`, `handlePostMergeRebase`, `DispatchPrRebaseStatus`, the post-merge-rebase dedup map (`alreadyDispatchedPostMergeRebase` / `markPostMergeRebaseDispatched` / `resetPostMergeRebaseDedup`) | | `workflow/trust.ts` | `isTrustedUser` | | `workflow/pr-flow.ts` | `handleIssueClosed`, `handlePrDependencyMarkers`, `handleStackedRebaseCascade`, the trigger-bus slot (`setTriggerBus` / `resetTriggerBus` / private `_triggerBus`), private `agentsForCleanup` | | `workflow/issue-unassign.ts` | `handleIssueUnassigned` | ### Moved into existing `slash-commands.ts` - `ApplyReadyStackCommandOpts` interface + `applyReadyStackCommand` impl ### Deleted - `apps/server/src/domain/workflow/event-handlers.ts` (0 LOC after the moves) - `apps/server/src/domain/workflow/event-handlers.test.ts` (tests redistributed alongside their functions) - The `legacyHandle*` aliases in `main.ts:121-123` — replaced by plain-named imports ### Updated callers ~15 files across `background/`, `domain/dispatch/`, `domain/flows-yaml/`, `domain/views/`, `domain/workflow/`, `http/`, `main.ts`. ## Test plan - [x] Pure refactor — zero behavioural change - [x] `bun run typecheck` clean - [x] Full server suite: 3271 pass / 0 fail (same count as before — refactor, not feature) - [x] `bun run biome check` clean (the single warning is in a pre-existing `apps/server/src/infrastructure/agent-env-sync/render-for-instance.ts` `stat` import, untouched here) - [ ] CI green on this PR Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

claude-desktop added 1 commit

2026-05-11 02:29:12 +00:00

refactor(workflow): split event-handlers.ts into topical modules; delete legacy* aliases

qa / dockerfile (pull_request) Successful in 8s

Details

qa / i18n-string-check (pull_request) Successful in 9s

Details

qa / sql-layer-check (pull_request) Successful in 11s

Details

qa / db-schema (pull_request) Successful in 21s

Details

qa / qa-1 (pull_request) Successful in 2m26s

Details

qa / qa (pull_request) Successful in 0s

Details

cc92966d1f

Closes #1096.
Closes #1095.

The 1100-LOC `event-handlers.ts` god-module is split into five focused
files matching the actual concern boundaries. The three `legacy*` aliases
in `main.ts` (Phase 4 / #1095) are dropped during the move — their
function targets now live in `workflow/pr-flow.ts` with their plain
names, so the prFlow capability impls import directly.

### New modules

| File | Exports |
|---|---|
| `workflow/review-detection.ts` | `fetchPrDispatchLabels`, `latestVerdict`, `detectOutstandingChangeRequest` (+ private `parseClosesIssue`, `fetchLinkedIssueLabels`) |
| `workflow/dispatch.ts` | `dispatchIssueForAgent`, `dispatchPrRebase`, `handlePostMergeRebase`, `DispatchPrRebaseStatus`, the post-merge-rebase dedup map (`alreadyDispatchedPostMergeRebase` / `markPostMergeRebaseDispatched` / `resetPostMergeRebaseDedup`) |
| `workflow/trust.ts` | `isTrustedUser` |
| `workflow/pr-flow.ts` | `handleIssueClosed`, `handlePrDependencyMarkers`, `handleStackedRebaseCascade`, the trigger-bus slot (`setTriggerBus` / `resetTriggerBus` / private `_triggerBus`), private `agentsForCleanup` |
| `workflow/issue-unassign.ts` | `handleIssueUnassigned` |

### Moved-into-slash-commands.ts (existing file)
- `ApplyReadyStackCommandOpts` interface + `applyReadyStackCommand` impl

### Deleted
- `apps/server/src/domain/workflow/event-handlers.ts` (0 LOC after the moves)
- `apps/server/src/domain/workflow/event-handlers.test.ts` (tests redistributed alongside their functions)
- The `legacyHandle*` aliases in `main.ts:121-123` — replaced by plain-named imports

### Updated callers (15+ files)
`background/janitor.ts`, `background/tail-pr-rebase-watchdog.ts`,
`domain/dispatch/registry.ts`, `domain/dispatch/redispatch-hook.test.ts`,
`domain/flows-yaml/{live-capabilities, types}.ts`, six op files
(`apply_ready_stack_command`, `cancel_tasks`, `detect_change_request`,
`handle_post_merge_rebase`, `is_trusted_user`, `pr_dependency_markers`,
`propagate_dependencies`), `domain/views/board.ts`,
`domain/workflow/{breakdown, post-ci, post-merge, slash-commands}.test.ts`,
`http/webhook.ts`, `main.ts`.

Pure refactor — zero behavioural change. Full server suite: 3271 pass / 0 fail.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

claude-desktop requested review from reviewer

2026-05-11 02:29:20 +00:00

claude-desktop commented

2026-05-11 02:31:07 +00:00

Author

Collaborator

Closing in favour of PR #1100 + follow-up

Discovered during overnight Phase 6 work that code-lead agent independently filed PR #1100 (feat(flows-yaml): inline legacy* prFlow callbacks; split cleanup_issue op) at 01:41 UTC. That PR:

Closes #1095 with the same scope (delete the three legacy* aliases + delete the three functions from event-handlers.ts)
Also adds a focused cleanup_issue op that makes the previously-invisible cleanup half of handleIssueClosed explicit (nicer than my approach, which left it inlined in the propagateIssueClosed capability adapter)
CI green, reviewer APPROVED with one comment-fix nit

My PR #1101 here combines Phase 4 (#1095) + Phase 6 (#1096 file split) into one 31-file diff that conflicts with #1100. CI on this PR is stuck in the runner queue at the moment.

Cleaner path:

Land #1100 (Phase 4 — small, focused, already reviewed)
Reopen Phase 6 as a fresh PR once #1100 is on main — the remaining work shrinks because three of the functions I was moving will already be gone
Close this PR + branch

Closing.

## Closing in favour of PR #1100 + follow-up Discovered during overnight Phase 6 work that `code-lead` agent independently filed PR #1100 (`feat(flows-yaml): inline legacy* prFlow callbacks; split cleanup_issue op`) at 01:41 UTC. That PR: - Closes #1095 with the same scope (delete the three `legacy*` aliases + delete the three functions from `event-handlers.ts`) - Also adds a focused `cleanup_issue` op that makes the previously-invisible cleanup half of `handleIssueClosed` explicit (nicer than my approach, which left it inlined in the `propagateIssueClosed` capability adapter) - CI green, reviewer APPROVED with one comment-fix nit My PR #1101 here combines Phase 4 (#1095) + Phase 6 (#1096 file split) into one 31-file diff that conflicts with #1100. CI on this PR is stuck in the runner queue at the moment. Cleaner path: 1. Land #1100 (Phase 4 — small, focused, already reviewed) 2. Reopen Phase 6 as a fresh PR once #1100 is on `main` — the remaining work shrinks because three of the functions I was moving will already be gone 3. Close this PR + branch Closing.

claude-desktop closed this pull request

2026-05-11 02:31:29 +00:00

reviewer approved these changes

2026-05-11 02:37:30 +00:00

reviewer left a comment

APPROVED ✅

Clean, mechanical refactor — zero behavioural changes detected.

What was verified

Import correctness (31 files)
Every file that previously imported from event-handlers now imports from the correct new module. No stale event-handlers import paths remain.

Export completeness (16 symbols)
All 16 previously-exported symbols are accounted for, each in exactly one new home, with no duplicates or silent drops:

Symbol	New module
`dispatchIssueForAgent`, `dispatchPrRebase`, `handlePostMergeRebase`, dedup-map trio	`workflow/dispatch.ts`
`fetchPrDispatchLabels`, `latestVerdict`, `detectOutstandingChangeRequest`	`workflow/review-detection.ts`
`handleIssueClosed`, `handlePrDependencyMarkers`, `handleStackedRebaseCascade`, trigger-bus slot	`workflow/pr-flow.ts`
`handleIssueUnassigned`	`workflow/issue-unassign.ts`
`isTrustedUser`	`workflow/trust.ts`
`applyReadyStackCommand`	`workflow/slash-commands.ts`

event-handlers.ts deletion
Fully deleted (1083 lines removed). No stub/barrel remnant.

main.ts legacy alias cleanup
The three legacyHandle* aliases at lines 121–123 are removed; buildLiveCapabilitiesDeps now wires up plain-named imports directly from their new modules. All three wiring sites (handlePrDependencyMarkers, handleStackedRebaseCascade, handleIssueClosed) verified correct.

Test redistribution
event-handlers.test.ts renamed to dispatch.test.ts; review-detection.test.ts added; issue-unassign.test.ts, slash-commands.test.ts, post-merge.test.ts updated. Coverage is preserved — same symbols, same scenarios, new homes.

CI: ✅ green (cc92966)
Behavioural delta: none

## APPROVED ✅ Clean, mechanical refactor — zero behavioural changes detected. ### What was verified **Import correctness (31 files)** Every file that previously imported from `event-handlers` now imports from the correct new module. No stale `event-handlers` import paths remain. **Export completeness (16 symbols)** All 16 previously-exported symbols are accounted for, each in exactly one new home, with no duplicates or silent drops: | Symbol | New module | |--------|-----------| | `dispatchIssueForAgent`, `dispatchPrRebase`, `handlePostMergeRebase`, dedup-map trio | `workflow/dispatch.ts` | | `fetchPrDispatchLabels`, `latestVerdict`, `detectOutstandingChangeRequest` | `workflow/review-detection.ts` | | `handleIssueClosed`, `handlePrDependencyMarkers`, `handleStackedRebaseCascade`, trigger-bus slot | `workflow/pr-flow.ts` | | `handleIssueUnassigned` | `workflow/issue-unassign.ts` | | `isTrustedUser` | `workflow/trust.ts` | | `applyReadyStackCommand` | `workflow/slash-commands.ts` | **`event-handlers.ts` deletion** Fully deleted (1083 lines removed). No stub/barrel remnant. **`main.ts` legacy alias cleanup** The three `legacyHandle*` aliases at lines 121–123 are removed; `buildLiveCapabilitiesDeps` now wires up plain-named imports directly from their new modules. All three wiring sites (`handlePrDependencyMarkers`, `handleStackedRebaseCascade`, `handleIssueClosed`) verified correct. **Test redistribution** `event-handlers.test.ts` renamed to `dispatch.test.ts`; `review-detection.test.ts` added; `issue-unassign.test.ts`, `slash-commands.test.ts`, `post-merge.test.ts` updated. Coverage is preserved — same symbols, same scenarios, new homes. **CI**: ✅ green (`cc92966`) **Behavioural delta**: none

reviewer reopened this pull request

2026-05-11 02:37:58 +00:00

charles referenced this pull request from a commit

2026-05-11 02:59:34 +00:00

test(workflow): restore 4 test suites for the moved helpers (review feedback for #1102)

claude-desktop commented

2026-05-11 08:19:39 +00:00

Author

Collaborator

Superseded by #1102 (same split — dispatch.ts, review-detection.ts, trust.ts, issue-unassign.ts, pr-flow.ts all landed on main) combined with #1100 (inlined the legacy* prFlow callbacks this PR also wanted to drop). Rebasing produces 9 add/add + content conflicts on files main now owns; no salvageable unique work. Closing.

Superseded by #1102 (same split — `dispatch.ts`, `review-detection.ts`, `trust.ts`, `issue-unassign.ts`, `pr-flow.ts` all landed on main) combined with #1100 (inlined the `legacy*` `prFlow` callbacks this PR also wanted to drop). Rebasing produces 9 add/add + content conflicts on files main now owns; no salvageable unique work. Closing.

claude-desktop closed this pull request

2026-05-11 08:19:42 +00:00