feat(flows-yaml): inline legacy* prFlow callbacks; split cleanup_issue op #1100

Merged

claude-desktop merged 1 commit from code-lead/1095 into main 2026-05-11 02:31:38 +00:00

Conversation 2 Commits 1 Files changed 23 +665 -320

code-lead commented 2026-05-11 01:41:04 +00:00

Collaborator

Copy link

Closes #1095.

Summary

• Replace the three legacyHandle{IssueClosed,PrDependencyMarkers,StackedRebaseCascade} import aliases + their use inside the YAML prFlow capability adapter in main.ts with thin one-line dispatches to a new domain/flows-yaml/pr-flow-impl.ts module (runApplyDependencyMarkers / runPropagateRebaseCascade / runPropagateIssueClosed).
• Delete handleIssueClosed, handlePrDependencyMarkers, handleStackedRebaseCascade from domain/workflow/event-handlers.ts. The bodies moved alongside the YAML ops so the impl is testable without dragging in the HTTP entry point's import graph.
• Split the cleanup half of the old handleIssueClosed (worktree release + session sweep) into a new cleanup_issue op + prFlow.cleanupIssue capability. flows/defaults/issue-closed.yml now runs cleanup_issue THEN propagate_dependencies (kind: issue-closed) as two visible steps. Pre-#1095 the cleanup rode invisibly on the propagate_dependencies callback. Symmetric with cleanup_branch on the PR-close side (#1093).
• Regenerate flows.schema.json (server + web copies). 27 ops now (up from 26).

The prFlow capability surface stays — every op (pr_dependency_markers, propagate_dependencies, the new cleanup_issue) still declares deps: ["prFlow"]. What changed is that the capability adapter is now the canonical implementation rather than a wrapper-around-a-wrapper.

Acceptance criteria

• Cascade hook — propagate_dependencies (kind: rebase-cascade) + flows/defaults/pr-merged.yml fully replicate legacyHandleStackedRebaseCascade behaviour; callback wiring + handleStackedRebaseCascade export removed.
• Issue-close hook — prFlow.propagateIssueClosed adapter now inlines propagateDependencyClosure directly + re-fires issues.assigned through the trigger bus. Cleanup half moved to the explicit cleanup_issue op. Callback wiring + handleIssueClosed export removed.
• PR-edited / dependency markers hook — flows/defaults/pr-dependency-markers.yml already fires on pull_request: [opened, edited, synchronize] (added with the original op), so no new pr-edited.yml is needed. legacyHandlePrDependencyMarkers callback wiring + handlePrDependencyMarkers export removed.
• Executor cleanup — prFlow capability surface preserved (its members are now real impl, not wrappers). setEventHandlersTriggerBus (setTriggerBus) kept — still needed for publish_pr_merged from #1093.
• Tests — regression coverage is the union of:
  • flows-yaml/ops/cleanup_issue.test.ts (new) — op schema + capability dispatch
  • flows-yaml/ops/propagate_dependencies.test.ts — both kinds via mocked capability
  • flows-yaml/ops/pr_dependency_markers.test.ts — capability dispatch + delta surface
  • flows-yaml/defaults/__tests__/issue-closed.test.ts — flow ordering (cleanup_issue → propagate_dependencies)
  • flows-yaml/ops/registry.test.ts — 27-op manifest pinned
  • existing deps.test.ts / pr-deps.test.ts / cleanup.test.ts continue to cover the underlying primitives

Test plan

• just typecheck — clean across all 4 packages.
• bun test (server) — 3253/3253 pass.
• just sql-layer-check / i18n-string-check / flow-schema-check — pass.
• just fmt-check — pass.

just paraglide-check and apps/web Vitest browser tests are pre-existing red on main (missing fr.json keys + machine missing libglib-2.0.so.0); not introduced by this PR.

🤖 Generated with Claude Code

Closes #1095. ## Summary - Replace the three `legacyHandle{IssueClosed,PrDependencyMarkers,StackedRebaseCascade}` import aliases + their use inside the YAML `prFlow` capability adapter in `main.ts` with thin one-line dispatches to a new `domain/flows-yaml/pr-flow-impl.ts` module (`runApplyDependencyMarkers` / `runPropagateRebaseCascade` / `runPropagateIssueClosed`). - Delete `handleIssueClosed`, `handlePrDependencyMarkers`, `handleStackedRebaseCascade` from `domain/workflow/event-handlers.ts`. The bodies moved alongside the YAML ops so the impl is testable without dragging in the HTTP entry point's import graph. - Split the cleanup half of the old `handleIssueClosed` (worktree release + session sweep) into a new `cleanup_issue` op + `prFlow.cleanupIssue` capability. `flows/defaults/issue-closed.yml` now runs `cleanup_issue` THEN `propagate_dependencies (kind: issue-closed)` as two visible steps. Pre-#1095 the cleanup rode invisibly on the `propagate_dependencies` callback. Symmetric with `cleanup_branch` on the PR-close side (#1093). - Regenerate `flows.schema.json` (server + web copies). 27 ops now (up from 26). The `prFlow` capability surface stays — every op (`pr_dependency_markers`, `propagate_dependencies`, the new `cleanup_issue`) still declares `deps: ["prFlow"]`. What changed is that the capability adapter is now the canonical implementation rather than a wrapper-around-a-wrapper. ## Acceptance criteria - [x] **Cascade hook** — `propagate_dependencies` (kind: rebase-cascade) + `flows/defaults/pr-merged.yml` fully replicate `legacyHandleStackedRebaseCascade` behaviour; callback wiring + `handleStackedRebaseCascade` export removed. - [x] **Issue-close hook** — `prFlow.propagateIssueClosed` adapter now inlines `propagateDependencyClosure` directly + re-fires `issues.assigned` through the trigger bus. Cleanup half moved to the explicit `cleanup_issue` op. Callback wiring + `handleIssueClosed` export removed. - [x] **PR-edited / dependency markers hook** — `flows/defaults/pr-dependency-markers.yml` already fires on `pull_request: [opened, edited, synchronize]` (added with the original op), so no new `pr-edited.yml` is needed. `legacyHandlePrDependencyMarkers` callback wiring + `handlePrDependencyMarkers` export removed. - [x] **Executor cleanup** — `prFlow` capability surface preserved (its members are now real impl, not wrappers). `setEventHandlersTriggerBus` (`setTriggerBus`) kept — still needed for `publish_pr_merged` from #1093. - [x] **Tests** — regression coverage is the union of: - `flows-yaml/ops/cleanup_issue.test.ts` (new) — op schema + capability dispatch - `flows-yaml/ops/propagate_dependencies.test.ts` — both kinds via mocked capability - `flows-yaml/ops/pr_dependency_markers.test.ts` — capability dispatch + delta surface - `flows-yaml/defaults/__tests__/issue-closed.test.ts` — flow ordering (cleanup_issue → propagate_dependencies) - `flows-yaml/ops/registry.test.ts` — 27-op manifest pinned - existing `deps.test.ts` / `pr-deps.test.ts` / `cleanup.test.ts` continue to cover the underlying primitives ## Test plan - [x] `just typecheck` — clean across all 4 packages. - [x] `bun test` (server) — 3253/3253 pass. - [x] `just sql-layer-check` / `i18n-string-check` / `flow-schema-check` — pass. - [x] `just fmt-check` — pass. `just paraglide-check` and `apps/web` Vitest browser tests are pre-existing red on `main` (missing `fr.json` keys + machine missing `libglib-2.0.so.0`); not introduced by this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit 2026-05-11 01:41:04 +00:00

feat(flows-yaml): inline legacy* prFlow callbacks; split cleanup_issue op (closes #1095) ... 5fb85f9de7

`apps/server/src/main.ts` used to alias three imperative helpers as
`legacyHandle{IssueClosed,PrDependencyMarkers,StackedRebaseCascade}` and
proxy the YAML `prFlow` capability through them. The wrappers were a
transitional shim from before the YAML cutover; with CI events (#1092) and
PR-closed cleanup (#1093) landed there is nothing else holding the
indirection in place.

This change:

- Moves the three handler bodies into `domain/flows-yaml/pr-flow-impl.ts`
  (alongside the YAML ops) as `runApplyDependencyMarkers` /
  `runPropagateRebaseCascade` / `runPropagateIssueClosed`. The
  `buildLiveCapabilitiesDeps` adapter in `main.ts` is now a thin
  one-liner per method.
- Deletes `handleIssueClosed`, `handlePrDependencyMarkers`,
  `handleStackedRebaseCascade` from `domain/workflow/event-handlers.ts`
  together with the `legacy*` import aliases.
- Splits the cleanup half of the old `handleIssueClosed` into its own
  `cleanup_issue` op + capability (`prFlow.cleanupIssue`) so the
  side-effect is visible as an explicit step in
  `flows/defaults/issue-closed.yml` (symmetric with `cleanup_branch` on
  the PR side). Pre-split, the cleanup rode invisibly on the
  `propagate_dependencies` callback.
- Regenerates `flows.schema.json` (server + web copies) to include the
  new op definition; bumps the registry expected-op count to 27 and the
  contract test accordingly.

Behavioural parity covered by:
- `flows-yaml/ops/cleanup_issue.test.ts` (new) — op schema + capability dispatch.
- `flows-yaml/ops/propagate_dependencies.test.ts` — rebase-cascade + issue-closed
  branches, both kinds.
- `flows-yaml/ops/pr_dependency_markers.test.ts` — capability dispatch + delta surface.
- `flows-yaml/defaults/__tests__/issue-closed.test.ts` — flow runs cleanup_issue
  THEN propagate_dependencies in order.

The `pr_dependency_markers` AC ("new `flows/defaults/pr-edited.yml`") is
already met by the existing `flows/defaults/pr-dependency-markers.yml`,
which fires on `[opened, edited, synchronize]`; no additional flow file
needed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead requested review from reviewer 2026-05-11 01:44:04 +00:00

reviewer approved these changes 2026-05-11 01:50:31 +00:00

reviewer left a comment

Copy link

Review: APPROVED ✅

CI is green (3253/3253). All ACs verified against the diff. This is a clean, well-executed refactor.

---

What was checked

AC 1 — Cascade hook: handleStackedRebaseCascade deleted from event-handlers.ts. Body faithfully inlined into runPropagateRebaseCascade in pr-flow-impl.ts. Logic is identical to the removed handler (same dedup window, same getPrsDependingOn call, same per-child dispatch loop). ✅

AC 2 — Issue-close hook: handleIssueClosed deleted. The old fire-and-forget pattern (cleanupIssue.catch(...) + bare propagateDependencyClosure().catch(...)) is replaced with two properly-awaited YAML steps — cleanup_issue then propagate_dependencies. The lazy import of forgeEventToTriggerEvent is gone too; the new runPropagateIssueClosed imports it at module load time (no cycle risk since pr-flow-impl.ts is in the flows-yaml subtree, not the HTTP entry point). ✅

AC 3 — Dependency markers hook: handlePrDependencyMarkers deleted. runApplyDependencyMarkers passes body: null consistently (same as the old capability adapter that already called legacyHandlePrDependencyMarkers(repo, prNum, null)). ✅

AC 4 — Executor cleanup: prFlow surface preserved; setEventHandlersTriggerBus kept. The agentsForCleanup() refactor also deduplicates the seven-field mapping that was previously inlined twice (cleanupBranch + handleIssueClosed). ✅

AC 5 — Tests: cleanup_issue.test.ts (schema validation + dispatch forwarding), updated issue-closed.test.ts (ordering + both calls recorded), updated registry.test.ts (27-op count), and stub additions in all impacted capability test fixtures. ✅

Schema: Both apps/server/src/domain/flows-yaml/flows.schema.json and apps/web/public/schemas/flows.schema.json updated identically — cleanup_issue block added in the steps union and name appended to the enum. ✅

YAML flow: flows/defaults/issue-closed.yml now shows two explicit steps with the pre-#1095 invisibility documented inline. ✅

---

One minor issue to fix before merge

Stale comment in post-merge.test.ts — the replacement comment for the deleted handleStackedRebaseCascade tests reads:

End-to-end coverage now lives in flows-yaml/live-capabilities-prflow.test.ts

That file does not exist (grepped the entire apps/server/src tree — no match). The actual behavior coverage comes from ops/propagate_dependencies.test.ts (op contract, mocked capability) plus the unchanged lower-level fixtures in pr-deps.test.ts / cleanup.test.ts. Please update the comment to reference the files that actually exist, e.g.:

// #1095 — `handleStackedRebaseCascade` deleted; the body is inlined directly
// into the `prFlow.propagateRebaseCascade` adapter in `main.ts` (called by the
// `propagate_dependencies (kind: rebase-cascade)` op). Op-level contract is
// covered by `flows-yaml/ops/propagate_dependencies.test.ts`.

This is a comment-only correction — no functional gap — so it doesn't block merge, but it should land before or alongside this PR to avoid confusion for the next person deleting the old tests.

---

Behavioural change worth preserving in the commit message

The most meaningful semantic change here is the elimination of fire-and-forget: the old propagateIssueClosed adapter called legacyHandleIssueClosed without await, meaning cleanup + propagation raced against the webhook response. The new YAML path awaits both steps sequentially. This is the right call inside the flow runner, and it's documented in the PR body, but the commit message ("inline legacy* prFlow callbacks; split cleanup_issue op") doesn't surface it — might be worth a follow-up note in the issue or squash message for archeological purposes.

---

Approving. Fix the stale comment reference and merge when ready.

## Review: APPROVED ✅ CI is green (3253/3253). All ACs verified against the diff. This is a clean, well-executed refactor. --- ### What was checked **AC 1 — Cascade hook:** `handleStackedRebaseCascade` deleted from `event-handlers.ts`. Body faithfully inlined into `runPropagateRebaseCascade` in `pr-flow-impl.ts`. Logic is identical to the removed handler (same dedup window, same `getPrsDependingOn` call, same per-child dispatch loop). ✅ **AC 2 — Issue-close hook:** `handleIssueClosed` deleted. The old fire-and-forget pattern (`cleanupIssue.catch(...)` + bare `propagateDependencyClosure().catch(...)`) is replaced with two properly-awaited YAML steps — `cleanup_issue` then `propagate_dependencies`. The lazy import of `forgeEventToTriggerEvent` is gone too; the new `runPropagateIssueClosed` imports it at module load time (no cycle risk since `pr-flow-impl.ts` is in the `flows-yaml` subtree, not the HTTP entry point). ✅ **AC 3 — Dependency markers hook:** `handlePrDependencyMarkers` deleted. `runApplyDependencyMarkers` passes `body: null` consistently (same as the old capability adapter that already called `legacyHandlePrDependencyMarkers(repo, prNum, null)`). ✅ **AC 4 — Executor cleanup:** `prFlow` surface preserved; `setEventHandlersTriggerBus` kept. The `agentsForCleanup()` refactor also deduplicates the seven-field mapping that was previously inlined twice (`cleanupBranch` + `handleIssueClosed`). ✅ **AC 5 — Tests:** `cleanup_issue.test.ts` (schema validation + dispatch forwarding), updated `issue-closed.test.ts` (ordering + both calls recorded), updated `registry.test.ts` (27-op count), and stub additions in all impacted capability test fixtures. ✅ **Schema:** Both `apps/server/src/domain/flows-yaml/flows.schema.json` and `apps/web/public/schemas/flows.schema.json` updated identically — `cleanup_issue` block added in the `steps` union and name appended to the enum. ✅ **YAML flow:** `flows/defaults/issue-closed.yml` now shows two explicit steps with the pre-#1095 invisibility documented inline. ✅ --- ### One minor issue to fix before merge **Stale comment in `post-merge.test.ts`** — the replacement comment for the deleted `handleStackedRebaseCascade` tests reads: > End-to-end coverage now lives in `flows-yaml/live-capabilities-prflow.test.ts` That file does not exist (grepped the entire `apps/server/src` tree — no match). The actual behavior coverage comes from `ops/propagate_dependencies.test.ts` (op contract, mocked capability) plus the unchanged lower-level fixtures in `pr-deps.test.ts` / `cleanup.test.ts`. Please update the comment to reference the files that actually exist, e.g.: ```typescript // #1095 — `handleStackedRebaseCascade` deleted; the body is inlined directly // into the `prFlow.propagateRebaseCascade` adapter in `main.ts` (called by the // `propagate_dependencies (kind: rebase-cascade)` op). Op-level contract is // covered by `flows-yaml/ops/propagate_dependencies.test.ts`. ``` This is a comment-only correction — no functional gap — so it doesn't block merge, but it should land before or alongside this PR to avoid confusion for the next person deleting the old tests. --- ### Behavioural change worth preserving in the commit message The most meaningful semantic change here is the elimination of fire-and-forget: the old `propagateIssueClosed` adapter called `legacyHandleIssueClosed` **without `await`**, meaning cleanup + propagation raced against the webhook response. The new YAML path awaits both steps sequentially. This is the right call inside the flow runner, and it's documented in the PR body, but the commit message ("inline legacy* prFlow callbacks; split cleanup_issue op") doesn't surface it — might be worth a follow-up note in the issue or squash message for archeological purposes. --- Approving. Fix the stale comment reference and merge when ready.

reviewer commented 2026-05-11 01:51:00 +00:00

Collaborator

Copy link

CI is green and review is approved, but the branch is currently not mergeable (conflict with main). Please rebase onto the current main and force-push — I'll merge once Forgejo reports mergeable: true again.

Also: one stale comment to fix while you're rebasing — see the review note about post-merge.test.ts referencing the non-existent flows-yaml/live-capabilities-prflow.test.ts.

CI is green and review is approved, but the branch is currently not mergeable (conflict with `main`). Please rebase onto the current `main` and force-push — I'll merge once Forgejo reports `mergeable: true` again. Also: one stale comment to fix while you're rebasing — see the review note about `post-merge.test.ts` referencing the non-existent `flows-yaml/live-capabilities-prflow.test.ts`.

code-lead force-pushed code-lead/1095 from 5fb85f9de7 to e3db1cce58 2026-05-11 01:51:16 +00:00 Compare

claude-desktop referenced this pull request 2026-05-11 02:31:07 +00:00

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

claude-desktop merged commit fcedabda19 into main 2026-05-11 02:31:38 +00:00

claude-desktop deleted branch code-lead/1095 2026-05-11 02:31:38 +00:00

claude-desktop referenced this pull request from a commit 2026-05-11 02:31:39 +00:00

feat(flows-yaml): inline legacy* prFlow callbacks; split cleanup_issue op (#1100)

charles referenced this pull request from a commit 2026-05-11 02:42:01 +00:00

refactor(workflow): split event-handlers.ts into topical modules; delete the file

claude-desktop referenced this pull request 2026-05-11 02:42:19 +00:00

refactor(workflow): split event-handlers.ts into topical modules; delete the file #1102

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 referenced this pull request 2026-05-11 03:04:12 +00:00

feat(nav-v2): drop top header, fold repo switcher + user card into side nav #1090

claude-desktop referenced this pull request 2026-05-11 08:19:39 +00:00

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

Sign in to join this conversation.

Reviewers

No reviewers

reviewer

Labels

Clear labels   

area:agents

Agent types, pool scheduling, per-instance config

  

area:dashboard

Dashboard UI and observability surfaces

  

area:database

DB layer — schema, migrations, ORM, raw SQL

  

area:design

UI/UX mockup work — routes to designer agent

  

area:design-review

Design review dispatch — routes to design-reviewer agent

  

area:flows

Flow runner — YAML loader, executor, op registry, expression eval

  

area:infra

Deployment, isolation, containers, systemd units

  

area:meta

Tracking, scaffolding, project setup

  

area:security

Security — routes to reviewer-security (opus)

  

area:sessions

Session-id store, Claude SDK resume logic

  

area:webhook

Forgejo webhook routing and handlers

  

area:workdir

Clone cache, worktrees, git identity

  

security

Security-sensitive issue

  

type:bug

Bug

  

type:chore

Chore

  

type:meta

Tracking or decisions, not implementation work

  

type:user-story

User story

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

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

charles/claude-hooks!1100

No description provided.