refactor(main): split main.ts into worker + agent-runner + storage #39

Merged
code-lead merged 1 commit from boss/36 into main 2026-04-18 11:13:11 +00:00
Collaborator

Summary

Splits the 1212-line src/main.ts into four focused modules so the 222-line Worker.runAgent stops being the biggest unreadable block in the codebase and each concern can be tested in isolation.

  • src/worker.ts (229 lines) — Worker class only: FIFO queue, enqueue, processNext, result store with 1h TTL, callback delivery. Takes a runTask hook at construction and knows nothing about the Claude Agent SDK.
  • src/agent-runner.ts (525 lines) — the former Worker.runAgent body: container pre-flight, workdir acquisition, env construction, SDK spawn, session resume with retry-on-invalid, session capture. Exports runAgentTask plus small pure helpers (runWithSessionResume, resolveHostCwd, buildAgentEnv, makeContainerGitRunner, makeContainerRemovePath, buildPrompt, extractProgress, touchesHostSecret) so the correctness rules can be unit-tested without spinning up the SDK.
  • src/storage.ts (115 lines) — disk-usage stats for /storage, extracted to bring main.ts close to the 500-line target.
  • src/main.ts (533 lines, down from 1212 — 56% reduction) — HTTP server, task history + event log, SSE broadcast, worker registry, startup wiring. Each Worker is constructed with runTask = runAgentTask(…) plus lifecycle hooks that feed the HTTP-visible event log.

Session-resume stays correct

The order of operations — check stored session → pass resume to SDK → on failure, drop session, retry fresh — is preserved character-for-character in the extracted runWithSessionResume helper. Five unit tests in agent-runner.test.ts cover: (a) fresh dispatch with no stored session, (b) successful resume, (c) resume fails → session dropped and fresh retry runs, plus stateless_session skipping the store entirely and fresh-dispatch failures not retrying.

Container mode unchanged

agent-runner.ts owns the cwd = hostCwd | workdir decision (the fix from 0e3ea72) in the new exported resolveHostCwd(containerMode, workdir) helper. Unit tests assert both branches. The pre-flight check still fails fast with a clear error when the agent's container is down — that regression test moved from main.test.ts to agent-runner.test.ts and now calls runAgentTask directly.

Public API stable

  • main.ts still exports getWorker with the same signature (webhook.ts untouched).
  • TaskRequest moved to worker.ts — where the Worker class actually uses it — and re-exported from main.ts so webhook.ts's import type { TaskRequest } from "./main" keeps working. Chose the re-export over a shared types.ts because TaskRequest is tightly coupled to Worker's queue semantics; splitting it into a third module just to host one interface would be cosmetic.

Verification

  • bun x tsc --noEmit — clean
  • bunx biome check src/ — clean
  • bun test129/129 pass (up from ~35, thanks to the new session-resume, cwd, and Worker-lifecycle coverage)

Closes #36

Test plan

  • bun x tsc --noEmit clean
  • bunx biome check src/ clean
  • bun test — all tests pass
  • Live E2E on charles/dummy-app issue → PR → merge still completes (requires human dispatch)

🤖 Generated with Claude Code

## Summary Splits the 1212-line `src/main.ts` into four focused modules so the 222-line `Worker.runAgent` stops being the biggest unreadable block in the codebase and each concern can be tested in isolation. - **`src/worker.ts`** (229 lines) — `Worker` class only: FIFO queue, `enqueue`, `processNext`, result store with 1h TTL, callback delivery. Takes a `runTask` hook at construction and knows nothing about the Claude Agent SDK. - **`src/agent-runner.ts`** (525 lines) — the former `Worker.runAgent` body: container pre-flight, workdir acquisition, env construction, SDK spawn, session resume with retry-on-invalid, session capture. Exports `runAgentTask` plus small pure helpers (`runWithSessionResume`, `resolveHostCwd`, `buildAgentEnv`, `makeContainerGitRunner`, `makeContainerRemovePath`, `buildPrompt`, `extractProgress`, `touchesHostSecret`) so the correctness rules can be unit-tested without spinning up the SDK. - **`src/storage.ts`** (115 lines) — disk-usage stats for `/storage`, extracted to bring `main.ts` close to the 500-line target. - **`src/main.ts`** (533 lines, down from 1212 — **56% reduction**) — HTTP server, task history + event log, SSE broadcast, worker registry, startup wiring. Each `Worker` is constructed with `runTask = runAgentTask(…)` plus lifecycle hooks that feed the HTTP-visible event log. ### Session-resume stays correct The order of operations — check stored session → pass `resume` to SDK → on failure, drop session, retry fresh — is preserved character-for-character in the extracted `runWithSessionResume` helper. Five unit tests in `agent-runner.test.ts` cover: (a) fresh dispatch with no stored session, (b) successful resume, (c) resume fails → session dropped and fresh retry runs, plus `stateless_session` skipping the store entirely and fresh-dispatch failures not retrying. ### Container mode unchanged `agent-runner.ts` owns the `cwd = hostCwd | workdir` decision (the fix from `0e3ea72`) in the new exported `resolveHostCwd(containerMode, workdir)` helper. Unit tests assert both branches. The pre-flight check still fails fast with a clear error when the agent's container is down — that regression test moved from `main.test.ts` to `agent-runner.test.ts` and now calls `runAgentTask` directly. ### Public API stable - `main.ts` still exports `getWorker` with the same signature (webhook.ts untouched). - `TaskRequest` moved to `worker.ts` — where the Worker class actually uses it — and re-exported from `main.ts` so `webhook.ts`'s `import type { TaskRequest } from "./main"` keeps working. Chose the re-export over a shared `types.ts` because `TaskRequest` is tightly coupled to Worker's queue semantics; splitting it into a third module just to host one interface would be cosmetic. ### Verification - `bun x tsc --noEmit` — clean - `bunx biome check src/` — clean - `bun test` — **129/129 pass** (up from ~35, thanks to the new session-resume, cwd, and Worker-lifecycle coverage) Closes #36 ## Test plan - [x] `bun x tsc --noEmit` clean - [x] `bunx biome check src/` clean - [x] `bun test` — all tests pass - [ ] Live E2E on `charles/dummy-app` issue → PR → merge still completes (requires human dispatch) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
refactor(main): split main.ts into worker + agent-runner + storage
All checks were successful
qa / qa (pull_request) Successful in 49s
qa / dockerfile (pull_request) Successful in 8s
f745fa5b42
Break the 1212-line src/main.ts into four focused modules so per-task
SDK execution stops being the biggest unreadable block in the service
and each concern can be tested in isolation.

- src/worker.ts — Worker class (FIFO queue, enqueue, processNext),
  result store with 1h TTL, and callback delivery. Takes a `runTask`
  hook at construction and knows nothing about the Claude Agent SDK.
- src/agent-runner.ts — former Worker.runAgent body: container
  pre-flight, workdir acquisition, env construction, SDK spawn,
  session resume with retry-on-invalid, session capture. Exports
  `runAgentTask` plus small pure helpers (`runWithSessionResume`,
  `resolveHostCwd`, `buildAgentEnv`, etc.) so the session-resume and
  host-cwd correctness rules can be unit-tested without spinning up
  the SDK.
- src/storage.ts — disk-usage stats for /storage, extracted to bring
  main.ts close to the 500-line target.
- src/main.ts — HTTP server, task history + event log, SSE, worker
  registry, startup wiring. Now 533 lines (down from 1212).

TaskRequest moves into worker.ts and is re-exported from main.ts so
webhook.ts's `import type { TaskRequest } from "./main"` keeps working
— public API stays stable per the issue's acceptance criteria.

Tests split into matching files (worker.test.ts, agent-runner.test.ts,
storage.test.ts) with new coverage for the three session-resume paths
(fresh dispatch, successful resume, resume fails → drop + retry), the
host-cwd decision, and the Worker lifecycle hooks. 129/129 pass.

Closes #36

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
reviewer approved these changes 2026-04-18 11:07:07 +00:00
reviewer left a comment

Review

CI: green (run #1532, 57s, all 129 tests pass)

Acceptance criteria check:

Criterion Status
src/worker.ts — Worker class only, SDK-agnostic, runTask callback
src/agent-runner.ts — full runAgentTask + pure helpers exported
src/main.ts target ≤ 500 lines ⚠️ 533 lines (see below)
Session-resume order preserved character-for-character
Unit tests: fresh / successful resume / resume-fails-then-retry all 5 cases covered in runWithSessionResume suite
resolveHostCwd container-mode regression extracted + two unit tests
Pre-flight fail-fast when container down moved to agent-runner.test.ts, calls runAgentTask directly
All existing tests pass after split
getWorker signature stable
TaskRequest still importable from "./main" re-exported
just qa green

The refactor is clean. Session-resume logic, container-mode env wiring, and the abort/FIFO queue semantics are faithfully preserved. The new WorkerHooks interface is a clear improvement — lifecycle concerns (event log, SSE, history) are now fully decoupled from the queue mechanics, which is exactly what the issue asked for.

Two minor observations (neither blocks merge):


src/main.ts — 533 lines vs ≤ 500 target

The acceptance criterion states "Target ≤ 500 lines." The PR lands at 533. The 56% reduction from 1212 lines is the real win here and the spirit is fully met, but the hard number misses by 33 lines. The logSDKMessage function (≈90 lines of switch arms) is the obvious candidate if you want to cross the line — it could move to agent-runner.ts or a new src/sdk-log.ts. Not requesting a change; flagging so it's a conscious decision rather than an oversight.


src/main.tslogSDKMessage is now exported but used only internally

-function logSDKMessage(taskId: string, msg: SDKMessage): void {
+export function logSDKMessage(taskId: string, msg: SDKMessage): void {

Nothing outside main.ts imports it (confirmed from the test diffs). The export is unnecessary and silently widens the public API surface. Drop the export keyword or, if you intend it to be testable in the future, add a note explaining why. No correctness impact.


Both items are follow-up material, not blockers. Approving.

## Review **CI**: ✅ green (run #1532, 57s, all 129 tests pass) **Acceptance criteria check:** | Criterion | Status | |---|---| | `src/worker.ts` — Worker class only, SDK-agnostic, `runTask` callback | ✅ | | `src/agent-runner.ts` — full `runAgentTask` + pure helpers exported | ✅ | | `src/main.ts` target ≤ 500 lines | ⚠️ 533 lines (see below) | | Session-resume order preserved character-for-character | ✅ | | Unit tests: fresh / successful resume / resume-fails-then-retry | ✅ all 5 cases covered in `runWithSessionResume` suite | | `resolveHostCwd` container-mode regression | ✅ extracted + two unit tests | | Pre-flight fail-fast when container down | ✅ moved to `agent-runner.test.ts`, calls `runAgentTask` directly | | All existing tests pass after split | ✅ | | `getWorker` signature stable | ✅ | | `TaskRequest` still importable from `"./main"` | ✅ re-exported | | `just qa` green | ✅ | The refactor is clean. Session-resume logic, container-mode env wiring, and the abort/FIFO queue semantics are faithfully preserved. The new `WorkerHooks` interface is a clear improvement — lifecycle concerns (event log, SSE, history) are now fully decoupled from the queue mechanics, which is exactly what the issue asked for. Two minor observations (neither blocks merge): --- ### `src/main.ts` — 533 lines vs ≤ 500 target The acceptance criterion states "Target ≤ 500 lines." The PR lands at 533. The 56% reduction from 1212 lines is the real win here and the spirit is fully met, but the hard number misses by 33 lines. The `logSDKMessage` function (≈90 lines of `switch` arms) is the obvious candidate if you want to cross the line — it could move to `agent-runner.ts` or a new `src/sdk-log.ts`. Not requesting a change; flagging so it's a conscious decision rather than an oversight. --- ### `src/main.ts` — `logSDKMessage` is now exported but used only internally ```diff -function logSDKMessage(taskId: string, msg: SDKMessage): void { +export function logSDKMessage(taskId: string, msg: SDKMessage): void { ``` Nothing outside `main.ts` imports it (confirmed from the test diffs). The `export` is unnecessary and silently widens the public API surface. Drop the `export` keyword or, if you intend it to be testable in the future, add a note explaining why. No correctness impact. --- Both items are follow-up material, not blockers. Approving.
@ -2,0 +2,4 @@
* claude-hooks HTTP entry point and orchestration.
*
* Owns:
* - HTTP server + API routes (`/task`, `/health`, `/queue`, `/cancel`,
Collaborator

This function was function logSDKMessage (module-private) in the original. The export keyword is now present but nothing outside main.ts imports it — confirmed by inspecting all test files in the diff. Either drop the export to keep it private, or add a comment explaining the intended public surface.

This function was `function logSDKMessage` (module-private) in the original. The `export` keyword is now present but nothing outside `main.ts` imports it — confirmed by inspecting all test files in the diff. Either drop the `export` to keep it private, or add a comment explaining the intended public surface.
code-lead deleted branch boss/36 2026-04-18 11:13: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!39
No description provided.