charles/claude-hooks

Fork 0

refactor(main): extract route handlers and adopt Hono for dispatch #104

Merged

code-lead merged 2 commits from refactor/main-route-handlers into main

2026-04-19 20:55:45 +00:00

claude-desktop commented

2026-04-19 20:38:39 +00:00

Collaborator

Summary

Two-step refactor in one branch:

Extract each of handleRequest's 14 routes into its own named handler function (commit 1). handleRequest drops from a 282-line if-else to a thin dispatcher.
Replace the hand-rolled dispatcher with Hono (commit 2). Path params come from c.req.param("id") instead of url.pathname.slice(…), method dispatch + 404 are framework-provided, and the route table is one app.get/post per route.

Zero behavior change. Public entry (handleRequest(req)) and Bun.serve wiring stay identical; tests need no updates.

Why now

The Agent pool milestone (#53) adds dashboard CRUD for agent instances — GET /agents, POST /agents, PUT /agents/:name, DELETE /agents/:name, pagination, validation. Doing that on top of the hand-rolled if-else would hurt. Switching before those routes land means they come in idiomatic Hono from day one.

Dependencies

hono — only addition. 184 ms install on Bun; single package (no transitive bloat to flag).

Shape (post-Hono)

const app = new Hono();

app.get("/health", () => handleHealth());
app.post("/task", (c) => handleTaskSubmit(c.req.raw));
app.get("/task/:id", (c) => handleTaskGet(c.req.param("id")));
app.post("/cancel", (c) => handleCancel(c.req.raw));
app.post("/reset", (c) => handleReset(c.req.raw));
app.get("/queue", () => handleQueue());
app.post("/webhook/forgejo", (c) => handleForgejoWebhook(c.req.raw));
app.get("/events", () => handleEvents());
app.get("/storage", () => handleStorage());
app.get("/history", () => handleHistory());
app.get("/history/:id", (c) => handleHistoryById(c.req.param("id")));
app.get("/", () => handleDashboard());
app.get("/dashboard", () => handleDashboard());
app.get("/manifest.webmanifest", () => handleManifest());
app.get("/icon.svg", () => handleIcons("icon.svg"));
app.get("/icon-maskable.svg", () => handleIcons("icon-maskable.svg"));
app.notFound((c) => c.json({ error: "not found" }, 404));

export const handleRequest = (req: Request): Promise<Response> =>
  Promise.resolve(app.fetch(req));

What stays stable

Public entry: handleRequest(req) exported with the same signature — main.test.ts (20 tests through this entry) passes unchanged; Bun.serve({ fetch: handleRequest }) works.
Handlers: each is a plain function returning Response. Hono is only the dispatcher; handler bodies are portable if we ever swap back.
Forgejo webhook: routes to handleForgejoWebhook(c.req.raw) — the raw native Request flows through untouched, so signature verification (which reads req.text()) still works.

Handler signature cleanups

Now that Hono extracts path params:

handleTaskGet(url: URL) → handleTaskGet(id: string)
handleHistoryById(url: URL) → handleHistoryById(id: string)
handleIcons(url: URL) → handleIcons(filename: string)

Test plan

just qa — 313 pass, 0 fail across both commits
main.test.ts (20 tests covering /health, /queue, /task, /cancel, unknown routes) passes via the unchanged handleRequest entry
webhook.test.ts (42 tests) — direct calls to handleForgejoWebhook, unchanged

🤖 Generated with Claude Code

## Summary Two-step refactor in one branch: 1. **Extract** each of `handleRequest`'s 14 routes into its own named handler function (commit 1). `handleRequest` drops from a 282-line if-else to a thin dispatcher. 2. **Replace** the hand-rolled dispatcher with **Hono** (commit 2). Path params come from `c.req.param("id")` instead of `url.pathname.slice(…)`, method dispatch + 404 are framework-provided, and the route table is one `app.get/post` per route. Zero behavior change. Public entry (`handleRequest(req)`) and `Bun.serve` wiring stay identical; tests need no updates. ## Why now The Agent pool milestone (#53) adds dashboard CRUD for agent instances — `GET /agents`, `POST /agents`, `PUT /agents/:name`, `DELETE /agents/:name`, pagination, validation. Doing that on top of the hand-rolled if-else would hurt. Switching before those routes land means they come in idiomatic Hono from day one. ## Dependencies - `hono` — only addition. 184 ms install on Bun; single package (no transitive bloat to flag). ## Shape (post-Hono) ```ts const app = new Hono(); app.get("/health", () => handleHealth()); app.post("/task", (c) => handleTaskSubmit(c.req.raw)); app.get("/task/:id", (c) => handleTaskGet(c.req.param("id"))); app.post("/cancel", (c) => handleCancel(c.req.raw)); app.post("/reset", (c) => handleReset(c.req.raw)); app.get("/queue", () => handleQueue()); app.post("/webhook/forgejo", (c) => handleForgejoWebhook(c.req.raw)); app.get("/events", () => handleEvents()); app.get("/storage", () => handleStorage()); app.get("/history", () => handleHistory()); app.get("/history/:id", (c) => handleHistoryById(c.req.param("id"))); app.get("/", () => handleDashboard()); app.get("/dashboard", () => handleDashboard()); app.get("/manifest.webmanifest", () => handleManifest()); app.get("/icon.svg", () => handleIcons("icon.svg")); app.get("/icon-maskable.svg", () => handleIcons("icon-maskable.svg")); app.notFound((c) => c.json({ error: "not found" }, 404)); export const handleRequest = (req: Request): Promise<Response> => Promise.resolve(app.fetch(req)); ``` ## What stays stable - **Public entry**: `handleRequest(req)` exported with the same signature — `main.test.ts` (20 tests through this entry) passes unchanged; `Bun.serve({ fetch: handleRequest })` works. - **Handlers**: each is a plain function returning `Response`. Hono is only the dispatcher; handler bodies are portable if we ever swap back. - **Forgejo webhook**: routes to `handleForgejoWebhook(c.req.raw)` — the raw native Request flows through untouched, so signature verification (which reads `req.text()`) still works. ## Handler signature cleanups Now that Hono extracts path params: - `handleTaskGet(url: URL)` → `handleTaskGet(id: string)` - `handleHistoryById(url: URL)` → `handleHistoryById(id: string)` - `handleIcons(url: URL)` → `handleIcons(filename: string)` ## Test plan - [x] `just qa` — 313 pass, 0 fail across both commits - [x] `main.test.ts` (20 tests covering `/health`, `/queue`, `/task`, `/cancel`, unknown routes) passes via the unchanged `handleRequest` entry - [x] `webhook.test.ts` (42 tests) — direct calls to `handleForgejoWebhook`, unchanged 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-desktop added 1 commit

2026-04-19 20:38:39 +00:00

refactor(main): extract HTTP route handlers from handleRequest

qa / qa (pull_request) Successful in 2m35s

Details

qa / dockerfile (pull_request) Successful in 8s

Details

8cef48e13e

Audit follow-up (PR #102). `handleRequest` was a 282-line if-else chain
covering 14 routes. Hard to reason about individually, hard to
unit-test one route without instantiating the whole dispatcher.

Extract each route body into its own top-level function. The names
read like a routing table when listed:

  handleHealth         GET  /health
  handleTaskSubmit     POST /task
  handleTaskGet        GET  /task/:id
  handleCancel         POST /cancel
  handleReset          POST /reset
  handleQueue          GET  /queue
  handleEvents         GET  /events
  handleStorage        GET  /storage
  handleHistory        GET  /history
  handleHistoryById    GET  /history/:id
  handleDashboard      GET  / | /dashboard
  handleManifest       GET  /manifest.webmanifest
  handleIcons          GET  /icon*.svg

`handleRequest` is now a thin dispatcher — 17 lines of
`(method, path) → handler` conditions plus the 404 fallback. The
public entry point stays identical, so main.test.ts (which calls
`handleRequest(req)` directly) and the `Bun.serve` wiring need no
changes.

No behavior change. 313 pass, 0 fail.

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

charles referenced this pull request from a commit

2026-04-19 20:42:55 +00:00

refactor(main): adopt Hono for HTTP routing

charles added 1 commit

2026-04-19 20:42:55 +00:00

refactor(main): adopt Hono for HTTP routing

qa / qa (pull_request) Successful in 3m33s

Details

qa / dockerfile (pull_request) Successful in 10s

Details

a06944f519

The hand-rolled dispatcher in PR #104 was a clean stepping stone —
now replace it with Hono so path params, method dispatch, and the 404
fallback come from a maintained framework instead of if-else chains.
Unblocks the upcoming /agents CRUD routes (Agent pool milestone #53).

- `package.json`: `hono` dep (only prod addition — 184ms install, no
  transitive footprint worth calling out).
- `src/main.ts`: build a `new Hono()` instance, wire each extracted
  handler via `app.get/post(path, handler)`. Path params like `:id`
  come from `c.req.param("id")` — killed the `url.pathname.slice(6)` /
  `slice(9)` dance. `/webhook/forgejo` routes to the existing handler
  via `c.req.raw` (unchanged native Request). `app.notFound` handles
  the fallback. Public entry `handleRequest` stays exported with the
  same `(req) => Promise<Response>` shape — `main.test.ts` and
  `Bun.serve({ fetch: handleRequest })` require zero changes.

- Minor handler signature cleanups now that Hono handles the path:
  `handleTaskGet(url)` → `handleTaskGet(id: string)`
  `handleHistoryById(url)` → `handleHistoryById(id: string)`
  `handleIcons(url)` → `handleIcons(filename: string)`

No behavior change. 313 pass, 0 fail.

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

claude-desktop changed title from ~~refactor(main): extract HTTP route handlers from handleRequest~~ to refactor(main): extract route handlers and adopt Hono for dispatch

2026-04-19 20:43:15 +00:00

charles requested review from reviewer

2026-04-19 20:53:05 +00:00

reviewer approved these changes

2026-04-19 20:54:53 +00:00

reviewer left a comment

Review: refactor(main) — extract route handlers + adopt Hono

CI: ✅ Green (run #1644, 3m45s, all 313 tests pass)

Summary

This is a clean, focused refactor. I reviewed the full src/main.ts diff, the package.json change, and the bun.lock update. Everything checks out.

Correctness

Each of the 14 routes is correctly wired to its handler in the Hono table. Method constraints (GET/POST) match the original if-else logic exactly.
handleForgejoWebhook receives c.req.raw — the native Request is passed through untouched, so HMAC signature verification (which calls req.text()) is unaffected. ✅
Path param extraction (c.req.param("id")) replaces the previous url.pathname.slice(…) slicing — cleaner and less error-prone. ✅
handleIcons receives a hardcoded string literal from its closure ("icon.svg", "icon-maskable.svg"), NOT a path param — no path-traversal surface. ✅
export const handleRequest = (req) => Promise.resolve(app.fetch(req)) correctly narrows Hono's (req, env?, ctx?) => Response | Promise<Response> signature to the (req) => Promise<Response> shape that Bun.serve and main.test.ts expect. ✅
Type re-exports (TaskRequest, TaskResult, WorkerConfig) preserved — existing consumers like webhook.ts stay source-compatible. ✅

Dependency

hono@^4.12.14 is the only new dependency. No transitive bloat. The API usage (app.get/post, c.req.raw, c.req.param, c.json, app.notFound) is stable public Hono API. ✅

Scope

Zero behavior change confirmed by the existing 62-test suite (20 main.test.ts + 42 webhook.test.ts) passing without modification. No scope creep — handler bodies are unchanged. ✅

LGTM.

## Review: refactor(main) — extract route handlers + adopt Hono **CI:** ✅ Green (run #1644, 3m45s, all 313 tests pass) --- ### Summary This is a clean, focused refactor. I reviewed the full `src/main.ts` diff, the `package.json` change, and the `bun.lock` update. Everything checks out. **Correctness** - Each of the 14 routes is correctly wired to its handler in the Hono table. Method constraints (GET/POST) match the original if-else logic exactly. - `handleForgejoWebhook` receives `c.req.raw` — the native `Request` is passed through untouched, so HMAC signature verification (which calls `req.text()`) is unaffected. ✅ - Path param extraction (`c.req.param("id")`) replaces the previous `url.pathname.slice(…)` slicing — cleaner and less error-prone. ✅ - `handleIcons` receives a hardcoded string literal from its closure (`"icon.svg"`, `"icon-maskable.svg"`), NOT a path param — no path-traversal surface. ✅ - `export const handleRequest = (req) => Promise.resolve(app.fetch(req))` correctly narrows Hono's `(req, env?, ctx?) => Response | Promise<Response>` signature to the `(req) => Promise<Response>` shape that `Bun.serve` and `main.test.ts` expect. ✅ - Type re-exports (`TaskRequest`, `TaskResult`, `WorkerConfig`) preserved — existing consumers like `webhook.ts` stay source-compatible. ✅ **Dependency** - `hono@^4.12.14` is the only new dependency. No transitive bloat. The API usage (`app.get/post`, `c.req.raw`, `c.req.param`, `c.json`, `app.notFound`) is stable public Hono API. ✅ **Scope** - Zero behavior change confirmed by the existing 62-test suite (20 `main.test.ts` + 42 `webhook.test.ts`) passing without modification. No scope creep — handler bodies are unchanged. ✅ LGTM.