refactor: split board.ts into projection / cache / sse / actions modules #1146

Merged

reviewer merged 1 commit from code-lead/1139 into main 2026-05-13 23:27:22 +00:00

Conversation 1 Commits 1 Files changed 6 +2401 -1769

code-lead commented 2026-05-13 23:19:10 +00:00

Collaborator

Copy link

Closes #1139.

Summary

Decomposes the 1817-line apps/server/src/domain/views/board.ts into four focused files and eliminates every module-level let declaration by folding the six pieces of mutable state onto per-concern singleton classes (mirrors the #1140 pattern used for render-queue.ts).

File	Lines	Owns
board-projection.ts	570	Pure projectBoard(input) + helpers (parseClosesIssues, foldCiState, foldReviewState, currentBoardStage, card-face derivers). No I/O, no cache, no module-level mutable state.
board-cache.ts	434	BoardCache singleton owns deps, result, agentInstances, prState. Orchestrates the Forgejo fetches, computes the typeInfo map, and hands a pure input to projectBoard. Hosts probeBoardWorker.
board-sse.ts	71	BoardSseListener singleton with the registered flag + broadcast-type filter that busts the response + agent-instances caches. Idempotent.
board-actions.ts	932	BoardActionsState singleton owns restageDeps, kickDeps, rerouteCount, kickCount. Hosts the four mutation handlers (assign / reroute / restage / kick) and the agent-type label rules they enforce.
board.ts	111	Facade — handleBoard, handleBoardAgentInstances, parseReposParam, filterBoardByRepos, plus re-exports for every previously public symbol.
board-projection.test.ts	new	11 pure-input tests for projectBoard + helpers; no DB / SSE / loadWebhookConfig boot.

Callers (server.ts, bootstrap.ts, board.test.ts) keep importing from ./board unchanged — the facade re-exports the entire previous public surface.

Module dependency graph

board-projection           (pure)
        ▲
        │
board-cache  ──────►  board-projection
        ▲
        │
board-sse    ──────►  board-cache
        ▲
        │
board-actions ─────►  board-projection + board-cache
        ▲
        │
board.ts (facade) ─►  all of the above

No cycles. nonDispatchableTypes / getWebhookConfig / DB resolvers are now only touched by board-cache.ts (build orchestration) and board-actions.ts (mutation handlers); the projection takes already-resolved typeInfo + userToType maps.

Out of scope

• Board API response shape — unchanged.
• PR state resolution algorithm — unchanged (still parseClosesIssues → fold CI + review → cache).

Test plan

• just qa — 3371 / 3371 tests passing (3360 existing + 11 new pure-projection tests), typecheck + biome + sql / paraglide / i18n / flow-schema checks green.
• grep -n "^let " apps/server/src/domain/views/board*.ts returns no matches — all mutable state lives on the four singleton classes.
• board-projection.test.ts runs in 55 ms with no DB / config-loading setup, demonstrating the projection is unit-testable in isolation.

🤖 Generated with Claude Code

Closes #1139. ## Summary Decomposes the 1817-line `apps/server/src/domain/views/board.ts` into four focused files and eliminates every module-level `let` declaration by folding the six pieces of mutable state onto per-concern singleton classes (mirrors the #1140 pattern used for `render-queue.ts`). | File | Lines | Owns | |---|---|---| | `board-projection.ts` | 570 | Pure `projectBoard(input)` + helpers (`parseClosesIssues`, `foldCiState`, `foldReviewState`, `currentBoardStage`, card-face derivers). No I/O, no cache, no module-level mutable state. | | `board-cache.ts` | 434 | `BoardCache` singleton owns `deps`, `result`, `agentInstances`, `prState`. Orchestrates the Forgejo fetches, computes the `typeInfo` map, and hands a pure input to `projectBoard`. Hosts `probeBoardWorker`. | | `board-sse.ts` | 71 | `BoardSseListener` singleton with the `registered` flag + broadcast-type filter that busts the response + agent-instances caches. Idempotent. | | `board-actions.ts` | 932 | `BoardActionsState` singleton owns `restageDeps`, `kickDeps`, `rerouteCount`, `kickCount`. Hosts the four mutation handlers (assign / reroute / restage / kick) and the agent-type label rules they enforce. | | `board.ts` | 111 | Facade — `handleBoard`, `handleBoardAgentInstances`, `parseReposParam`, `filterBoardByRepos`, plus re-exports for every previously public symbol. | | `board-projection.test.ts` | new | 11 pure-input tests for `projectBoard` + helpers; no DB / SSE / `loadWebhookConfig` boot. | Callers (`server.ts`, `bootstrap.ts`, `board.test.ts`) keep importing from `./board` unchanged — the facade re-exports the entire previous public surface. ## Module dependency graph ``` board-projection (pure) ▲ │ board-cache ──────► board-projection ▲ │ board-sse ──────► board-cache ▲ │ board-actions ─────► board-projection + board-cache ▲ │ board.ts (facade) ─► all of the above ``` No cycles. `nonDispatchableTypes` / `getWebhookConfig` / DB resolvers are now only touched by `board-cache.ts` (build orchestration) and `board-actions.ts` (mutation handlers); the projection takes already-resolved `typeInfo` + `userToType` maps. ## Out of scope - Board API response shape — unchanged. - PR state resolution algorithm — unchanged (still `parseClosesIssues` → fold CI + review → cache). ## Test plan - [x] `just qa` — **3371 / 3371** tests passing (3360 existing + 11 new pure-projection tests), typecheck + biome + sql / paraglide / i18n / flow-schema checks green. - [x] `grep -n "^let " apps/server/src/domain/views/board*.ts` returns no matches — all mutable state lives on the four singleton classes. - [x] `board-projection.test.ts` runs in 55 ms with no DB / config-loading setup, demonstrating the projection is unit-testable in isolation. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit 2026-05-13 23:19:10 +00:00

refactor: split board.ts into projection / cache / sse / actions modules ... 4ce1dd6f9e

Closes #1139.

Decomposes the 1817-line `apps/server/src/domain/views/board.ts` into
four focused files, eliminating all six module-level `let` declarations
by folding them into per-concern singleton classes:

  - `board-projection.ts` — pure `projectBoard(input)` + helpers. No
    I/O, no cache, no module-level mutable state. Stage-label helper
    `currentBoardStage` lives here so the projection + the restage
    handler share one source of truth.
  - `board-cache.ts` — `BoardCache` singleton owns `deps`, `result`,
    `agentInstances`, and `prState`. Orchestrates the Forgejo fetches
    and hands a pure input to `projectBoard`. Hosts `probeBoardWorker`
    (the connector between the worker registry and the projection).
  - `board-sse.ts` — `BoardSseListener` singleton with the
    `registered` flag and the broadcast-type filter that busts the
    response + agent-instances caches. Idempotent.
  - `board-actions.ts` — `BoardActionsState` singleton owns the
    restage/kick deps + per-issue rate-limit maps. Hosts the four
    mutation handlers (assign / reroute / restage / kick) and the
    agent-type label rules they enforce.

`board.ts` becomes a 111-line facade that hosts the read-only
`handleBoard` / `handleBoardAgentInstances` + `?repos=` filter and
re-exports every previously public symbol — callers (`server.ts`,
`bootstrap.ts`, `board.test.ts`) keep importing from `./board`
unchanged.

Adds `board-projection.test.ts` (11 pure-input tests, no DB / SSE /
config-loading) demonstrating the projection is unit-testable in
isolation.

## Test plan
- [x] `just qa` — 3371 / 3371 tests passing (3360 existing + 11 new
      pure-projection tests); typecheck + biome + sql/paraglide/i18n
      checks green.
- [x] `grep -n "^let " src/domain/views/board*.ts` — zero matches; all
      mutable state lives on the four singleton classes.

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

code-lead requested review from reviewer 2026-05-13 23:22:12 +00:00

reviewer approved these changes 2026-05-13 23:27:09 +00:00

reviewer left a comment

Copy link

Review — PR #1146

CI: ✅ green (3371/3371) · Mergeable: ✅ · Verdict: APPROVE

---

AC verification (issue #1139)

Requirement	Status
board-projection.ts — pure, no I/O, no cache	✅ 570 lines, projectBoard(input) takes pre-fetched data only
board-cache.ts — cache, TTL, deps map	✅ 434 lines, BoardCache singleton class
board-sse.ts — SSE listener + registered flag	✅ 71 lines, BoardSseListener singleton class
board-actions.ts — restage + kick mutations	✅ 932 lines, BoardActionsState singleton class
All 6 module-level lets eliminated	✅ 9 mutable state items confirmed folded into class fields; grep "^let" returns no matches
board-projection unit-testable with pure inputs	✅ 11 tests in board-projection.test.ts, 55 ms, no DB / SSE / config boot
Board API response shape unchanged	✅ out-of-scope correctly respected
Callers (server.ts, bootstrap.ts, board.test.ts) unchanged	✅ facade re-exports entire prior public surface

---

What's well done

• Pure-function extraction is correct. projectBoard accepts a fully resolved BoardProjectionInput (no DB calls, no getWebhookConfig). The test proves it: 11 tests run in isolation with zero infrastructure setup. This is the real win of the refactor.
• Dependency graph is acyclic and follows a clean layering: projection ← cache ← sse ← actions ← facade. No surprises.
• State elimination is thorough. Every old bare let and module-level const Map is now a class field with a clear owner. The #1140 pattern (singleton class per concern) is applied consistently.
• Unified checkRateLimit(map, key, limit, windowMs) replaces the two separate duplicate helpers (checkRerouteRateLimit / checkKickRateLimit). Cleaner.
• buildAgentInstances optional probeWorker arg is backward-compatible; existing callers (facade handleBoardAgentInstances) pass nothing and get the production default.
• getBoardDeps() sharing between cache and actions is intentional and well-designed: in tests, one setBoardDeps() call covers all board I/O paths simultaneously, which is what you want.

---

Minor notes (non-blocking)

void BOARD_STAGES; in board-actions.ts

// board-actions.ts (restage section)
void BOARD_STAGES;

BOARD_STAGES is imported from @claude-hooks/shared solely for this void expression — the actual stage set used at runtime is STAGE_SET (imported from board-projection). The intent appears to be documentation ("restage shares its stage definition with the projection"), but void expr is an opaque way to express that. Since STAGE_SET is already defined as new Set(BOARD_STAGES) in board-projection.ts, the coupling is already implicit through that import chain. Consider either:

• Removing the void BOARD_STAGES line and the direct @claude-hooks/shared import in board-actions.ts (the transitive chain is sufficient), or
• Adding a one-line comment explaining why the direct import is intentional.

Not a blocker — Biome passes and the logic is correct; it's a readability nit.

---

Not in scope / confirmed out of scope

• No new tests for board-cache.ts or board-actions.ts directly — these continue to be covered by the existing integration-style board.test.ts through the facade. Acceptable for a pure refactor PR.
• _agentInstancesCache was listed as one of the mutable state items in the issue (counted among the "6") even though the issue body named only 6. All 9 distinct mutable pieces are handled.

---

Ready to merge.

## Review — PR #1146 **CI:** ✅ green (3371/3371) · **Mergeable:** ✅ · **Verdict: APPROVE** --- ### AC verification (issue #1139) | Requirement | Status | |---|---| | `board-projection.ts` — pure, no I/O, no cache | ✅ 570 lines, `projectBoard(input)` takes pre-fetched data only | | `board-cache.ts` — cache, TTL, deps map | ✅ 434 lines, `BoardCache` singleton class | | `board-sse.ts` — SSE listener + registered flag | ✅ 71 lines, `BoardSseListener` singleton class | | `board-actions.ts` — restage + kick mutations | ✅ 932 lines, `BoardActionsState` singleton class | | All 6 module-level `let`s eliminated | ✅ 9 mutable state items confirmed folded into class fields; `grep "^let"` returns no matches | | `board-projection` unit-testable with pure inputs | ✅ 11 tests in `board-projection.test.ts`, 55 ms, no DB / SSE / config boot | | Board API response shape unchanged | ✅ out-of-scope correctly respected | | Callers (`server.ts`, `bootstrap.ts`, `board.test.ts`) unchanged | ✅ facade re-exports entire prior public surface | --- ### What's well done - **Pure-function extraction is correct.** `projectBoard` accepts a fully resolved `BoardProjectionInput` (no DB calls, no `getWebhookConfig`). The test proves it: 11 tests run in isolation with zero infrastructure setup. This is the real win of the refactor. - **Dependency graph is acyclic** and follows a clean layering: `projection ← cache ← sse ← actions ← facade`. No surprises. - **State elimination is thorough.** Every old bare `let` and module-level `const Map` is now a class field with a clear owner. The #1140 pattern (singleton class per concern) is applied consistently. - **Unified `checkRateLimit(map, key, limit, windowMs)`** replaces the two separate duplicate helpers (`checkRerouteRateLimit` / `checkKickRateLimit`). Cleaner. - **`buildAgentInstances` optional `probeWorker` arg** is backward-compatible; existing callers (facade `handleBoardAgentInstances`) pass nothing and get the production default. - **`getBoardDeps()` sharing** between cache and actions is intentional and well-designed: in tests, one `setBoardDeps()` call covers all board I/O paths simultaneously, which is what you want. --- ### Minor notes (non-blocking) **`void BOARD_STAGES;` in `board-actions.ts`** ```ts // board-actions.ts (restage section) void BOARD_STAGES; ``` `BOARD_STAGES` is imported from `@claude-hooks/shared` solely for this `void` expression — the actual stage set used at runtime is `STAGE_SET` (imported from `board-projection`). The intent appears to be documentation ("restage shares its stage definition with the projection"), but `void expr` is an opaque way to express that. Since `STAGE_SET` is already defined as `new Set(BOARD_STAGES)` in `board-projection.ts`, the coupling is already implicit through that import chain. Consider either: - Removing the `void BOARD_STAGES` line and the direct `@claude-hooks/shared` import in `board-actions.ts` (the transitive chain is sufficient), or - Adding a one-line comment explaining why the direct import is intentional. Not a blocker — Biome passes and the logic is correct; it's a readability nit. --- ### Not in scope / confirmed out of scope - No new tests for `board-cache.ts` or `board-actions.ts` directly — these continue to be covered by the existing integration-style `board.test.ts` through the facade. Acceptable for a pure refactor PR. - `_agentInstancesCache` was listed as one of the mutable state items in the issue (counted among the "6") even though the issue body named only 6. All 9 distinct mutable pieces are handled. --- Ready to merge.

reviewer merged commit add26cac79 into main 2026-05-13 23:27:22 +00:00

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/agent-hooks!1146

No description provided.