refactor(server): hexagonal architecture — folder reorg + ForgePort + ClaudeAgentPort #274

Merged
charles merged 7 commits from refactor/hexagonal-phase1-4 into main 2026-04-23 20:43:11 +00:00
Collaborator

Depends on #273 (contains the janitor fix this branch is based on). Rebase on main once #273 merges.

Scope

Six-commit stack that reorganises apps/server/src/ and introduces two hexagonal ports for the service's external dependencies. Groundwork for migrating Forgejo and Claude SDK callers off direct library imports.

Commits

SHA Phase What
4632630 1 Folder reorg — 42 modules moved into http/ domain/ infrastructure/ background/ shared/
da7043d 2 Port interfaces — ForgePort (22 methods, 7 domain shapes) + ClaudeAgentPort (runTaskAsyncIterable<TaskEvent>)
e2e518f 3a ForgejoAdapter implementing ForgePort, wrapping forgejo-api.ts
b4acea2 3b Migrate review-loop, slash-commands, main to ForgePort
f4b5230 3c Migrate foreman; extend port with ForgeDirEntry + ForgeIssue.htmlUrl
e311000 4 SdkClaudeAgent implementing ClaudeAgentPort, wrapping @anthropic-ai/claude-agent-sdk's query()

Folder layout after Phase 1

apps/server/src/
├── main.ts                      (entry, stays at root)
├── http/                        (webhook*, auth, sse)
├── domain/
│   ├── agent/                   (agent-runner, foreman, steer, mcp-config)
│   ├── dispatch/                (pool, registry)
│   ├── workflow/                (review-loop, cleanup, deps, slash-commands)
│   ├── views/                   (pipeline, board)
│   └── analytics/               (task-analytics, agents-health, skill-loader, token-economy)
├── infrastructure/
│   ├── agent/                   (claude-port, sdk-adapter)
│   ├── container/               (container*, container-reconcile, container-watchdog)
│   ├── database/                (db, task-store, sessions, settings)
│   ├── forge/                   (forgejo-api, forgejo-port, forgejo-adapter, labels)
│   ├── vcs/                     (workdir)
│   └── event-log, storage, billing
├── background/                  (worker, sweeper, janitor, redispatch-interrupted)
└── shared/                      (shutdown, config/webhook-config)

Consumer migrations

Forgejo (4/10):

  • review-loop.ts, slash-commands.ts, main.ts, foreman.ts

Forgejo (6/10 pending — recommend per-consumer follow-up PRs):

  • deps.ts, webhook-handlers.ts, pipeline.ts, board.ts, webhook-ci.ts, janitor.ts

Claude SDK (0/2 pending):

  • agent-runner.ts and foreman.ts still call query() directly. Migration requires Phase 5 (split agent-runner's multi-responsibility body).

Port highlights

ForgePort

Forges domain shapes with no Forgejo field names leaking:

interface ForgeIssue {
  number: number; title: string; body: string;
  state: "open" | "closed";
  labels: string[];           // was: { name: string }[]
  milestone: string | null;   // was: { title: string }
  assignees: string[];        // was: { login: string }[]
  author: string;             // was: user.login
  htmlUrl: string;
  createdAt: string; updatedAt: string; closedAt: string | null;
}

Token is bound at adapter construction — one adapter per agent-type.

ClaudeAgentPort

Single runTask(req): AsyncIterable<TaskEvent>. Seven event variants (assistant, user, result, system, tool_use, tool_progress, tool_summary), each carrying a raw: unknown escape hatch for callers that still need SDKMessage internals.

Checks

  • bunx tsc --noEmit -p apps/server/tsconfig.json — EXIT=0
  • bunx biome check apps/server/src — clean
  • bun --cwd apps/server test — 990 pass, 4 pre-existing fails (3 sweeper JSONL pruning + 1 foreman flaky test; all present on main)

What's explicitly NOT in this PR

  • No behavior change anywhere — pure reorg + new ports + 4 consumer swaps that preserve existing semantics
  • No consumer migration for the 6 Forgejo hot spots (pipeline, board, webhook-handlers, deps, webhook-ci, janitor) — each is high-touch enough to deserve its own review
  • No SDK consumer migration — agent-runner needs a split first
  • No deletion of forgejo-api.ts — still the implementation ForgejoAdapter wraps; will deprecate once all consumers migrate

Risk

  • Low for Phase 1 moves — git rename detection preserves history
  • Low for Phase 2 interfaces — pure additions, no imports touched
  • Low for Phase 3 adapter — ForgejoAdapter is new; nothing calls it except the 4 migrated consumers
  • Medium for the 4 migrated consumers — shape semantics change from raw Forgejo JSON to domain types. Tests updated. Fetch-spy integration tests still cover end-to-end.
  • Low for Phase 4 SDK adapter — SdkClaudeAgent is new, no consumers.

Follow-up backlog

  • Per-consumer Forgejo migration PRs (6 files)
  • Split foreman.ts into agent-logic + HTTP-handler halves (Phase 5a)
  • Migrate agent-runner.ts to SdkClaudeAgent (Phase 5b — highest-value SDK decoupling)
  • Delete forgejo-api.ts exports once zero consumers remain
Depends on #273 (contains the janitor fix this branch is based on). Rebase on main once #273 merges. ## Scope Six-commit stack that reorganises `apps/server/src/` and introduces two hexagonal ports for the service's external dependencies. Groundwork for migrating Forgejo and Claude SDK callers off direct library imports. ## Commits | SHA | Phase | What | |---|---|---| | `4632630` | 1 | Folder reorg — 42 modules moved into `http/` `domain/` `infrastructure/` `background/` `shared/` | | `da7043d` | 2 | Port interfaces — `ForgePort` (22 methods, 7 domain shapes) + `ClaudeAgentPort` (`runTask` → `AsyncIterable<TaskEvent>`) | | `e2e518f` | 3a | `ForgejoAdapter` implementing `ForgePort`, wrapping `forgejo-api.ts` | | `b4acea2` | 3b | Migrate `review-loop`, `slash-commands`, `main` to `ForgePort` | | `f4b5230` | 3c | Migrate `foreman`; extend port with `ForgeDirEntry` + `ForgeIssue.htmlUrl` | | `e311000` | 4 | `SdkClaudeAgent` implementing `ClaudeAgentPort`, wrapping `@anthropic-ai/claude-agent-sdk`'s `query()` | ## Folder layout after Phase 1 ``` apps/server/src/ ├── main.ts (entry, stays at root) ├── http/ (webhook*, auth, sse) ├── domain/ │ ├── agent/ (agent-runner, foreman, steer, mcp-config) │ ├── dispatch/ (pool, registry) │ ├── workflow/ (review-loop, cleanup, deps, slash-commands) │ ├── views/ (pipeline, board) │ └── analytics/ (task-analytics, agents-health, skill-loader, token-economy) ├── infrastructure/ │ ├── agent/ (claude-port, sdk-adapter) │ ├── container/ (container*, container-reconcile, container-watchdog) │ ├── database/ (db, task-store, sessions, settings) │ ├── forge/ (forgejo-api, forgejo-port, forgejo-adapter, labels) │ ├── vcs/ (workdir) │ └── event-log, storage, billing ├── background/ (worker, sweeper, janitor, redispatch-interrupted) └── shared/ (shutdown, config/webhook-config) ``` ## Consumer migrations **Forgejo (4/10):** - ✅ `review-loop.ts`, `slash-commands.ts`, `main.ts`, `foreman.ts` **Forgejo (6/10 pending — recommend per-consumer follow-up PRs):** - `deps.ts`, `webhook-handlers.ts`, `pipeline.ts`, `board.ts`, `webhook-ci.ts`, `janitor.ts` **Claude SDK (0/2 pending):** - `agent-runner.ts` and `foreman.ts` still call `query()` directly. Migration requires Phase 5 (split agent-runner's multi-responsibility body). ## Port highlights ### ForgePort Forges domain shapes with no Forgejo field names leaking: ```ts interface ForgeIssue { number: number; title: string; body: string; state: "open" | "closed"; labels: string[]; // was: { name: string }[] milestone: string | null; // was: { title: string } assignees: string[]; // was: { login: string }[] author: string; // was: user.login htmlUrl: string; createdAt: string; updatedAt: string; closedAt: string | null; } ``` Token is bound at adapter construction — one adapter per agent-type. ### ClaudeAgentPort Single `runTask(req): AsyncIterable<TaskEvent>`. Seven event variants (assistant, user, result, system, tool_use, tool_progress, tool_summary), each carrying a `raw: unknown` escape hatch for callers that still need SDKMessage internals. ## Checks - `bunx tsc --noEmit -p apps/server/tsconfig.json` — EXIT=0 - `bunx biome check apps/server/src` — clean - `bun --cwd apps/server test` — 990 pass, 4 pre-existing fails (3 sweeper JSONL pruning + 1 foreman flaky test; all present on `main`) ## What's explicitly NOT in this PR - No behavior change anywhere — pure reorg + new ports + 4 consumer swaps that preserve existing semantics - No consumer migration for the 6 Forgejo hot spots (pipeline, board, webhook-handlers, deps, webhook-ci, janitor) — each is high-touch enough to deserve its own review - No SDK consumer migration — agent-runner needs a split first - No deletion of `forgejo-api.ts` — still the implementation `ForgejoAdapter` wraps; will deprecate once all consumers migrate ## Risk - **Low** for Phase 1 moves — git rename detection preserves history - **Low** for Phase 2 interfaces — pure additions, no imports touched - **Low** for Phase 3 adapter — `ForgejoAdapter` is new; nothing calls it except the 4 migrated consumers - **Medium** for the 4 migrated consumers — shape semantics change from raw Forgejo JSON to domain types. Tests updated. Fetch-spy integration tests still cover end-to-end. - **Low** for Phase 4 SDK adapter — `SdkClaudeAgent` is new, no consumers. ## Follow-up backlog - Per-consumer Forgejo migration PRs (6 files) - Split `foreman.ts` into agent-logic + HTTP-handler halves (Phase 5a) - Migrate `agent-runner.ts` to `SdkClaudeAgent` (Phase 5b — highest-value SDK decoupling) - Delete `forgejo-api.ts` exports once zero consumers remain
feat(janitor): periodic reconciler — detect stuck issues/PRs/tasks and self-heal (#269)
Some checks failed
qa / qa (pull_request) Failing after 5m29s
qa / dockerfile (pull_request) Successful in 11s
5de6ba05f5
New module apps/server/src/janitor.ts with 7 rules:

- closes_keyword_drift: close issues referenced by Closes/Fixes/Resolves
  in a recently merged PR that Forgejo didn't auto-close
- design_approved_not_closed: close issues where design-reviewer posted
  '**Verdict**: APPROVED' but the skill missed the close step
- label_dispatch_miss: bounce area:design / area:design-review /
  area:dashboard routing labels that have no dispatch in the last 2h
- dependent_unblocked: re-fire assignee bounce for issues whose
  blockers are all closed but the propagator never ran
- zero_output_success: flag success tasks with 0 turns / null cost
- stale_idle_assigned: comment + bounce assignee on issues idle >30 min
- ready_to_merge: flag PRs that are mergeable + CI green + APPROVED

Config block janitor: { enabled, interval_ms, mode, rules } in
config/agents.json. Default mode is dry-run. GET /janitor/history
returns last 100 actions. Each action emits a janitor_action SSE
envelope. Per-rule per-target 6h idempotency guard.

Supporting changes:
- forgejo-api: patchIssue, listRepoLabels, addIssueLabels,
  removeIssueLabel, listClosedPullRequests, extend PullRequestDetail
  with merged/merged_at, make IssueListEntry labels id optional
- task-store: hasRecentDispatch, listZeroOutputSuccesses
- webhook-config: parse janitor: block
- shared: JanitorAction, JanitorReport, JanitorHistoryResponse types

42 unit tests, all green.
refactor: s11
Some checks failed
qa / qa (pull_request) Failing after 3m52s
qa / dockerfile (pull_request) Successful in 8s
cca21080f8
CLAUDE.md was 55.2k chars, past the 40k perf threshold. Keep the
navigation essentials at the root (overview, roles, modules table,
API table, tech stack, conventions, commands) and move detail
sections to per-topic files under docs/ linked from the root index.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(janitor): replace mock.module with impl seam; fix test-isolation leaks
All checks were successful
qa / qa (pull_request) Successful in 4m11s
qa / dockerfile (pull_request) Successful in 6s
97e9db3e8a
The s11 refactor moved dispatchByType / getWorker / broadcastSSE out of
main.ts into registry.ts / sse.ts. Several test files updated the module
specifier in their mock.module() calls, but three related leaks made
20 sibling-file tests fail in the full bun test run:

1. janitor.test.ts used mock.module("./forgejo-api") and ("./task-analytics")
   to stub its deps. Bun's mock.module is process-global — those stubs
   persisted through every later test file and broke deps.test.ts,
   webhook-handlers.test.ts pivot tests, propagateDependencyClosure, etc.
   Fix: add `impl` test seam to janitor.ts (same pattern as cleanup.ts)
   so the test mutates impl.* instead of touching module registry.

2. webhook-post-merge.test.ts was still mocking "./main" for dispatchByType
   (refactor missed this one) and also mocked ./main's broadcastSSE, both
   of which now live in ./registry / ./sse. Fix: point mock.module at the
   correct modules; drop the unused getWorker override (not exercised by
   handlePostMergeRebase).

3. webhook-assign-dedup / webhook-ci / force-merge.integration all mocked
   ./registry with both dispatchByType AND getWorker. getWorker is only
   actually exercised by webhook-assign-dedup's assign-dedup tests; the
   other two carried stale overrides that corrupted getWorker for every
   later test file (main.test.ts POST /cancel + POST /reset). Fix: drop
   the getWorker override where unused; in webhook-assign-dedup, keep the
   stub but fall back to the real function for names the test doesn't
   track, so main.test.ts's registered workers still resolve.

Plus: fix the stale mock.module("./main") → ("./registry") in
webhook-handlers.test.ts and refresh an outdated comment in main.test.ts.

Server test suite now: 990 pass / 4 fail — and the 4 remaining failures
(session JSONL pruning + foreman session CRUD) are pre-existing on main,
out of scope for this branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Group 42 flat modules into 6 responsibility-scoped folders to prepare for
hexagonal adapter extraction. No behavior change — pure file moves plus
import-path updates, mock.module specifier fixes, and 5 import.meta.dir
adjustments for files that moved deeper in the tree.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sketch the hexagonal port interfaces that domain code will consume instead
of forgejo-api.ts and @anthropic-ai/claude-agent-sdk. Interface-only
scaffolding — no adapters, no imports updated yet.

ForgePort covers 22 methods across issue/PR/comment/review/dep/CI/label/file
buckets with domain-pure shapes (ForgeIssue, ForgePullRequest, …).
ClaudeAgentPort exposes a single runTask() returning an AsyncIterable of
TaskEvents; SDKMessage never leaks across the boundary (escape hatch via
raw: unknown on each turn).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Concrete adapter that wraps forgejo-api.ts and maps Forgejo's raw REST
shapes into the domain-pure types defined in forgejo-port.ts.

Token is bound at construction — build one adapter per agent-type from
the token resolved by webhook-config.ts.

No consumers migrated in this commit; forgejo-api.ts remains the path of
record for the 10 existing importers. Consumer migrations will land as
separate per-module commits so each one is reviewable in isolation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switch three low-surface consumers from forgejo-api.ts direct calls to
ForgejoAdapter. Each call site now goes through the port, which means the
domain logic reads ForgeReview / ForgeIssue shapes instead of raw Forgejo
JSON.

review-loop.ts: guards now receive ForgeReview[] with lowercase state
enums (approved, changes_requested, comment, …). Tests updated to use
the domain shape when calling the pure counters directly; fetch-spy
integration tests untouched since the adapter still hits the same HTTP
paths.

slash-commands.ts: /raise-cap ack and miss comments go through the port.

main.ts: /redispatch probe issue fetch uses the port. Drops five unused
re-exports (createIssueComment, deleteReviewRequest, getPullRequest,
requestReviewers, updateIssueAssignees) that were imported but never
called.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Port additions:
- ForgeDirEntry shape for listDir (name, path, type, size, sha)
- htmlUrl field on ForgeIssue (needed by createIssue → breakdown summary link)
- readFile returns { content, sha } so callers can chain writeFile
- writeFile takes optional sha for update-vs-create discrimination

foreman.ts migration:
- /foreman/repos, /foreman/files, /foreman/specs/:name save,
  /foreman/breakdown-preview remote fetch, /foreman/create-issues all
  go through ForgejoAdapter
- spec save now surfaces a 502 on adapter failure instead of 500ing
  out of createOrUpdateRepoFile's throw (adapter swallows to bool)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
refactor(server): add SdkClaudeAgent implementing ClaudeAgentPort
All checks were successful
qa / qa (pull_request) Successful in 4m13s
qa / dockerfile (pull_request) Successful in 8s
e311000726
Concrete adapter that wraps @anthropic-ai/claude-agent-sdk's query() and
re-emits each SDKMessage as a domain-pure TaskEvent. SDKMessage is
preserved on event.raw as an escape hatch for callers that still need
SDK internals (e.g. tool_use_summary parsing for PR-URL extraction).

Port additions based on actual SDK surface:
- ResultEvent now carries resultText (needed for PR-URL matching against
  the final result payload that agent-runner consumes today)
- Added ToolUseEvent / ToolProgressEvent / ToolSummaryEvent to cover the
  SDK's tool-related message types — previously TaskEvent only covered
  user / assistant / result / system
- permissionMode values corrected: "bypassPermissions" replaces "bypass"
  to match SDKOptions.permissionMode

No consumers migrated — agent-runner.ts and foreman.ts still call query()
directly. Migration is Phase 5, which requires splitting agent-runner's
multi-responsibility body (container pre-flight + session resume + SDK
call + tool-policy + session persistence).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
refactor(server): address PR #274 review — fix shape lies, drop dead cache, harden parseRepo
All checks were successful
qa / qa (pull_request) Successful in 4m8s
qa / dockerfile (pull_request) Successful in 8s
bf70609576
Review flagged five correctness issues that would bite the first Phase 5
consumer migration. None change runtime behavior today (no consumer reads
the affected fields yet), but the port was advertising data the adapter
couldn't actually deliver from the narrow forgejo-api types.

### Shape mapper faithfulness

Widened forgejo-api.ts raw types so every field ForgePort promises is
actually declared on the underlying response:

- PullRequestSummary / PullRequestDetail / ClosedPullRequest — added
  state, base.ref, body, labels, assignees, requested_reviewers,
  mergeable, merged, draft, created_at, updated_at, merged_at
- PullReview — added submitted_at (was hardcoded to "" in adapter)
- WorkflowRunSummary — added name, head_sha, run_started_at, created_at
  (all were hardcoded to "" in toForgeWorkflowRun)
- IssueSummary / IssueListEntry — added user, html_url, created_at,
  closed_at, milestone, assignees so getIssue / listIssues return the
  full shape ForgeIssue advertises

ForgejoAdapter's shape mappers no longer need `as unknown as` casts —
types line up directly with the widened API.

### Dead label-id cache

ForgejoAdapter is instantiated per-call by every migrated consumer
(different tokens per agent type), so the instance-scoped cache was
always a miss. Dropped it; added a comment documenting that a TTL
cache is the right answer if we ever need one.

### parseRepo hardens on malformed input

parseRepo("foo") used to return `{owner: "foo", name: ""}` and silently
cause 404s downstream. Now throws with a clear error. Every current
caller pre-validates (regex or trusted source), so no caller is
affected.

### Stale path in force-merge PR comment

review-loop.ts posted `src/review-loop.ts` in its user-visible
force-merge comment; after the reorg the file is at
`src/domain/workflow/review-loop.ts`. Fixed.

### ToolUseEvent dropped

Declared by ClaudeAgentPort but never emitted — the SDK doesn't have a
top-level tool_use message type (tool uses are content blocks on the
assistant message, not separate SDKMessage entries). Removed from the
TaskEvent union so the port advertises only what the adapter actually
produces.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
charles force-pushed refactor/hexagonal-phase1-4 from bf70609576
All checks were successful
qa / qa (pull_request) Successful in 4m8s
qa / dockerfile (pull_request) Successful in 8s
to 989f1cb673
All checks were successful
qa / qa (pull_request) Successful in 4m5s
qa / dockerfile (pull_request) Successful in 9s
2026-04-23 20:36:36 +00:00
Compare
charles deleted branch refactor/hexagonal-phase1-4 2026-04-23 20:43:12 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
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!274
No description provided.