From 6bc403d88d9fec784e177aef369770a8b9180042 Mon Sep 17 00:00:00 2001 From: arkon Date: Sun, 15 Mar 2026 03:47:18 +0100 Subject: [PATCH] refactor: clean up case routes DRY violations, remove dead export, standardize reply API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract readLinkedCases() helper and resolveCasePath() to eliminate 6x duplicated linked-cases.json path construction and 5x duplicated file read/parse logic - Replace O(n) .some() duplicate check with O(1) Set.has() in case listing - Un-export isError() in types/api.ts (only used internally by getErrorMessage) - Standardize reply.status() → reply.code() in system-routes (Fastify canonical API) - Update CLAUDE.md: accurate frontend module listing, SSE event count (~106) Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 4 +- src/types/api.ts | 2 +- src/web/routes/case-routes.ts | 129 ++++++++++---------------------- src/web/routes/system-routes.ts | 6 +- 4 files changed, 47 insertions(+), 94 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4b8745ac..a87ce3b3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -109,7 +109,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph | **Infra** | `src/hooks-config.ts`, `src/push-store.ts`, `src/tunnel-manager.ts`, `src/image-watcher.ts`, `src/file-stream-manager.ts` | | | **Plan** | `src/plan-orchestrator.ts`, `src/prompts/*.ts`, `src/templates/claude-md.ts` | | | **Web** | `src/web/server.ts`, `src/web/sse-events.ts`, `src/web/routes/*.ts` (13 route modules incl. `ws-routes.ts` + barrel), `src/web/ports/*.ts`, `src/web/middleware/auth.ts`, `src/web/schemas.ts` | | -| **Frontend** | `src/web/public/app.js` (~2.6K lines, core) + 6 domain modules (`terminal-ui.js`, `respawn-ui.js`, `ralph-panel.js`, `settings-ui.js`, `panels-ui.js`, `session-ui.js`) + 5 existing modules (`ralph-wizard.js`, `api-client.js`, `subagent-windows.js`, `sw.js`, `input-cjk.js`) | | +| **Frontend** | `src/web/public/app.js` (~2.6K lines, core) + 5 infra modules (`constants.js`, `mobile-handlers.js`, `voice-input.js`, `notification-manager.js`, `keyboard-accessory.js`) + 6 domain modules (`terminal-ui.js`, `respawn-ui.js`, `ralph-panel.js`, `settings-ui.js`, `panels-ui.js`, `session-ui.js`) + 4 feature modules (`ralph-wizard.js`, `api-client.js`, `subagent-windows.js`, `input-cjk.js`) + `sw.js` | | | **Types** | `src/types/index.ts` → 13 domain files | See `@fileoverview` in index.ts | ★ = Large file (>50KB). All files have `@fileoverview` JSDoc — read that before diving in. @@ -166,7 +166,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L ### SSE Event Registry -~100 event types in `src/web/sse-events.ts` (backend) and `SSE_EVENTS` in `constants.js` (frontend). Both must be kept in sync. +~106 event types in `src/web/sse-events.ts` (backend) and `SSE_EVENTS` in `constants.js` (frontend). Both must be kept in sync. ### API Routes diff --git a/src/types/api.ts b/src/types/api.ts index 39959ca2..6961b315 100644 --- a/src/types/api.ts +++ b/src/types/api.ts @@ -117,7 +117,7 @@ export interface CaseInfo { * @param value The value to check * @returns True if the value is an Error instance */ -export function isError(value: unknown): value is Error { +function isError(value: unknown): value is Error { return value instanceof Error; } diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 45d34fac..745ef861 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -18,6 +18,28 @@ import { CASES_DIR, validatePathWithinBase } from '../route-helpers.js'; import { SseEvent } from '../sse-events.js'; import type { EventPort, ConfigPort } from '../ports/index.js'; +const LINKED_CASES_FILE = join(homedir(), '.codeman', 'linked-cases.json'); + +/** Read and parse linked-cases.json, returning empty object on missing/invalid file. */ +async function readLinkedCases(): Promise> { + try { + return JSON.parse(await fs.readFile(LINKED_CASES_FILE, 'utf-8')); + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { + console.warn('[Server] Failed to read linked cases:', err); + } + return {}; + } +} + +/** Resolve a case name to its directory path, checking linked cases if not in CASES_DIR. */ +async function resolveCasePath(name: string): Promise { + const casePath = join(CASES_DIR, name); + if (existsSync(casePath)) return casePath; + const linkedCases = await readLinkedCases(); + return linkedCases[name] ?? casePath; +} + export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & ConfigPort): void { // ═══════════════════════════════════════════════════════════════ // Case CRUD (list, create, link, detail, fix-plan) @@ -45,22 +67,15 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config } // Get linked cases - const linkedCasesFile = join(homedir(), '.codeman', 'linked-cases.json'); - try { - const linkedCases: Record = JSON.parse(await fs.readFile(linkedCasesFile, 'utf-8')); - for (const [name, path] of Object.entries(linkedCases)) { - // Only add if not already in cases (avoid duplicates) and path exists - if (!cases.some((c) => c.name === name) && existsSync(path)) { - cases.push({ - name, - path, - hasClaudeMd: existsSync(join(path, 'CLAUDE.md')), - }); - } - } - } catch (err) { - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { - console.warn('[Server] Failed to read linked cases:', err); + const linkedCases = await readLinkedCases(); + const existingNames = new Set(cases.map((c) => c.name)); + for (const [name, path] of Object.entries(linkedCases)) { + if (!existingNames.has(name) && existsSync(path)) { + cases.push({ + name, + path, + hasClaudeMd: existsSync(join(path, 'CLAUDE.md')), + }); } } @@ -126,15 +141,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config } // Load existing linked cases - const linkedCasesFile = join(homedir(), '.codeman', 'linked-cases.json'); - let linkedCases: Record = {}; - try { - linkedCases = JSON.parse(await fs.readFile(linkedCasesFile, 'utf-8')); - } catch (err) { - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { - console.warn('[Server] Failed to read linked cases:', err); - } - } + const linkedCases = await readLinkedCases(); // Check if name is already linked if (linkedCases[name]) { @@ -151,7 +158,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config if (!existsSync(codemanDir)) { mkdirSync(codemanDir, { recursive: true }); } - await fs.writeFile(linkedCasesFile, JSON.stringify(linkedCases, null, 2)); + await fs.writeFile(LINKED_CASES_FILE, JSON.stringify(linkedCases, null, 2)); ctx.broadcast(SseEvent.CaseLinked, { name, path: expandedPath }); return { success: true, data: { case: { name, path: expandedPath } } }; } catch (err) { @@ -166,34 +173,18 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case name'); } - // First check linked cases - const linkedCasesFile = join(homedir(), '.codeman', 'linked-cases.json'); - try { - const linkedCases: Record = JSON.parse(await fs.readFile(linkedCasesFile, 'utf-8')); - if (linkedCases[name]) { - const linkedPath = linkedCases[name]; - return { - name, - path: linkedPath, - hasClaudeMd: existsSync(join(linkedPath, 'CLAUDE.md')), - linked: true, - }; - } - } catch { - // ENOENT or parse errors - fall through to CASES_DIR check - } - - // Then check CASES_DIR - const casePath = join(CASES_DIR, name); + const casePath = await resolveCasePath(name); if (!existsSync(casePath)) { return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Case not found'); } + const linked = casePath !== join(CASES_DIR, name); return { name, path: casePath, hasClaudeMd: existsSync(join(casePath, 'CLAUDE.md')), + ...(linked && { linked: true }), }; }); @@ -206,21 +197,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config } // Get case path (check linked cases first, then CASES_DIR) - let casePath: string | null = null; - - const linkedCasesFile = join(homedir(), '.codeman', 'linked-cases.json'); - try { - const linkedCases: Record = JSON.parse(await fs.readFile(linkedCasesFile, 'utf-8')); - if (linkedCases[name]) { - casePath = linkedCases[name]; - } - } catch { - // ENOENT or parse errors - fall through to CASES_DIR - } - - if (!casePath) { - casePath = join(CASES_DIR, name); - } + const casePath = await resolveCasePath(name); const fixPlanPath = join(casePath, '@fix_plan.md'); @@ -321,23 +298,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config app.get('/api/cases/:caseName/ralph-wizard/files', async (req) => { const { caseName } = req.params as { caseName: string }; - let casePath = validatePathWithinBase(caseName, CASES_DIR); - if (!casePath) { + if (!validatePathWithinBase(caseName, CASES_DIR)) { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case name'); } - // Check linked cases if path doesn't exist - if (!existsSync(casePath)) { - const linkedCasesFile = join(homedir(), '.codeman', 'linked-cases.json'); - try { - const linkedCases: Record = JSON.parse(await fs.readFile(linkedCasesFile, 'utf-8')); - if (linkedCases[caseName]) { - casePath = linkedCases[caseName]; - } - } catch { - // No linked cases file - } - } + const casePath = await resolveCasePath(caseName); const wizardDir = join(casePath, 'ralph-wizard'); @@ -376,8 +341,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config // Cache disabled to ensure fresh prompts when starting new plan generations app.get('/api/cases/:caseName/ralph-wizard/file/:filePath', async (req, reply) => { const { caseName, filePath } = req.params as { caseName: string; filePath: string }; - let casePath = validatePathWithinBase(caseName, CASES_DIR); - if (!casePath) { + if (!validatePathWithinBase(caseName, CASES_DIR)) { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case name'); } @@ -386,18 +350,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config reply.header('Pragma', 'no-cache'); reply.header('Expires', '0'); - // Check linked cases if path doesn't exist - if (!existsSync(casePath)) { - const linkedCasesFile = join(homedir(), '.codeman', 'linked-cases.json'); - try { - const linkedCases: Record = JSON.parse(await fs.readFile(linkedCasesFile, 'utf-8')); - if (linkedCases[caseName]) { - casePath = linkedCases[caseName]; - } - } catch { - // No linked cases file - } - } + const casePath = await resolveCasePath(caseName); const wizardDir = join(casePath, 'ralph-wizard'); diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index e3f43771..4adf6834 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -709,7 +709,7 @@ export function registerSystemRoutes( for await (const chunk of req.raw) { totalSize += chunk.length; if (totalSize > MAX_SCREENSHOT_SIZE) { - reply.status(413); + reply.code(413); return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'File too large (max 10MB)'); } chunks.push(chunk as Buffer); @@ -794,12 +794,12 @@ export function registerSystemRoutes( const { name } = req.params as { name: string }; // Prevent path traversal if (name.includes('/') || name.includes('\\') || name.includes('..')) { - reply.status(400); + reply.code(400); return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid filename'); } const filepath = join(SCREENSHOTS_DIR, name); if (!existsSync(filepath)) { - reply.status(404); + reply.code(404); return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Screenshot not found'); } const ext = name.match(/\.(png|jpg|jpeg|webp|gif)$/i)?.[1]?.toLowerCase() ?? 'png';