BitSquare/Codeman
2
0
Fork 0
mirror of https://github.com/Ark0N/Codeman.git synced 2026-09-30 12:39:42 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix: address review findings from route extraction refactor

Browse Source 
- Add addSession() to SessionPort, replacing unsafe ReadonlyMap casts
- Restore getLightSessionsState() 1s TTL cache for GET /api/sessions
- Deduplicate AUTH_COOKIE_NAME, CASES_DIR, SETTINGS_PATH constants
- Fix eslint no-require-imports error in system-routes.ts
- Remove unused imports (homedir) from cleaned-up route modules

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
arkon
2026-03-01 03:26:08 +01:00
co-authored by Claude Opus 4.6
parent e05d507254
commit a27be18a5a
10 changed files with 61 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/web/middleware/auth.ts
+1 -1
View File
@@ -15,7 +15,7 @@ import { StaleExpirationMap } from '../../utils/index.js';
// Auth session cookie TTL (24h — matches autonomous run length)
const AUTH_SESSION_TTL_MS = 24 * 60 * 60 * 1000;
// Auth session cookie name
const AUTH_COOKIE_NAME = 'codeman_session';
export const AUTH_COOKIE_NAME = 'codeman_session';
// Max concurrent auth sessions
const MAX_AUTH_SESSIONS = 100;
// Max failed auth attempts per IP before rate-limiting
src/web/ports/config-port.ts
+1
View File
@@ -17,6 +17,7 @@ export interface ConfigPort {
getClaudeModeConfig(): Promise<{ claudeMode?: ClaudeMode; allowedTools?: string }>;
getDefaultClaudeMdPath(): Promise<string | undefined>;
getLightState(): unknown;
getLightSessionsState(): unknown[];
startTranscriptWatcher(sessionId: string, transcriptPath: string): void;
stopTranscriptWatcher(sessionId: string): void;
}
src/web/ports/session-port.ts
+1
View File
@@ -7,6 +7,7 @@ import type { Session } from '../../session.js';
export interface SessionPort {
readonly sessions: ReadonlyMap<string, Session>;
addSession(session: Session): void;
cleanupSession(sessionId: string, killMux?: boolean, reason?: string): Promise<void>;
setupSessionListeners(session: Session): Promise<void>;
persistSessionState(session: Session): void;
src/web/route-helpers.ts
+5
View File
@@ -6,12 +6,17 @@
*/
import { join } from 'node:path';
import { homedir } from 'node:os';
import { Session } from '../session.js';
import { ApiErrorCode, createErrorResponse } from '../types.js';
import { parseRalphLoopConfig, extractCompletionPhrase } from '../ralph-config.js';
import type { SessionPort } from './ports/session-port.js';
import type { EventPort } from './ports/event-port.js';
// Shared path constants used across route modules
export const CASES_DIR = join(homedir(), 'codeman-cases');
export const SETTINGS_PATH = join(homedir(), '.codeman', 'settings.json');
// Maximum hook data size (prevents oversized SSE broadcasts)
const MAX_HOOK_DATA_SIZE = 8 * 1024;
src/web/routes/case-routes.ts
+20 -21
View File
@@ -14,30 +14,29 @@ import { ApiErrorCode, createErrorResponse, getErrorMessage } from '../../types.
import { CreateCaseSchema, LinkCaseSchema } from '../schemas.js';
import { generateClaudeMd } from '../../templates/claude-md.js';
import { writeHooksConfig } from '../../hooks-config.js';
import { CASES_DIR } from '../route-helpers.js';
import type { EventPort, ConfigPort } from '../ports/index.js';
const casesDir = join(homedir(), 'codeman-cases');
export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & ConfigPort): void {
// ============ Case CRUD ============
app.get('/api/cases', async (): Promise<CaseInfo[]> => {
const cases: CaseInfo[] = [];
// Get cases from casesDir
// Get cases from CASES_DIR
try {
const entries = await fs.readdir(casesDir, { withFileTypes: true });
const entries = await fs.readdir(CASES_DIR, { withFileTypes: true });
for (const e of entries) {
if (e.isDirectory()) {
cases.push({
name: e.name,
path: join(casesDir, e.name),
hasClaudeMd: existsSync(join(casesDir, e.name, 'CLAUDE.md')),
path: join(CASES_DIR, e.name),
hasClaudeMd: existsSync(join(CASES_DIR, e.name, 'CLAUDE.md')),
});
}
}
} catch {
// casesDir may not exist yet
// CASES_DIR may not exist yet
}
// Get linked cases
@@ -70,11 +69,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
}
const { name, description } = result.data;
const casePath = join(casesDir, name);
const casePath = join(CASES_DIR, name);
// Security: Path traversal protection - use relative path check
const resolvedPath = resolve(casePath);
const resolvedBase = resolve(casesDir);
const resolvedBase = resolve(CASES_DIR);
const relPath = relative(resolvedBase, resolvedPath);
if (relPath.startsWith('..') || isAbsolute(relPath)) {
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case path');
@@ -120,8 +119,8 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
return createErrorResponse(ApiErrorCode.NOT_FOUND, `Folder not found: ${expandedPath}`);
}
// Check if case name already exists in casesDir
const casePath = join(casesDir, name);
// Check if case name already exists in CASES_DIR
const casePath = join(CASES_DIR, name);
if (existsSync(casePath)) {
return createErrorResponse(ApiErrorCode.ALREADY_EXISTS, 'A case with this name already exists in codeman-cases.');
}
@@ -177,11 +176,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
};
}
} catch {
// ENOENT or parse errors - fall through to casesDir check
// ENOENT or parse errors - fall through to CASES_DIR check
}
// Then check casesDir
const casePath = join(casesDir, name);
// Then check CASES_DIR
const casePath = join(CASES_DIR, name);
if (!existsSync(casePath)) {
return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Case not found');
@@ -198,7 +197,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
app.get('/api/cases/:name/fix-plan', async (req) => {
const { name } = req.params as { name: string };
// Get case path (check linked cases first, then casesDir)
// Get case path (check linked cases first, then CASES_DIR)
let casePath: string | null = null;
const linkedCasesFile = join(homedir(), '.codeman', 'linked-cases.json');
@@ -208,11 +207,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
casePath = linkedCases[name];
}
} catch {
// ENOENT or parse errors - fall through to casesDir
// ENOENT or parse errors - fall through to CASES_DIR
}
if (!casePath) {
casePath = join(casesDir, name);
casePath = join(CASES_DIR, name);
}
const fixPlanPath = join(casePath, '@fix_plan.md');
@@ -310,11 +309,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 = join(casesDir, caseName);
let casePath = join(CASES_DIR, caseName);
// Security: Path traversal protection - use relative path check
const resolvedCase = resolve(casePath);
const resolvedBase = resolve(casesDir);
const resolvedBase = resolve(CASES_DIR);
const relPath = relative(resolvedBase, resolvedCase);
if (relPath.startsWith('..') || isAbsolute(relPath)) {
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case name');
@@ -370,7 +369,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 = join(casesDir, caseName);
let casePath = join(CASES_DIR, caseName);
// Prevent browser caching - prompts change between plan generations
reply.header('Cache-Control', 'no-store, no-cache, must-revalidate');
@@ -379,7 +378,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
// Security: Path traversal protection for case name - use relative path check
const resolvedCase = resolve(casePath);
const resolvedBase = resolve(casesDir);
const resolvedBase = resolve(CASES_DIR);
const relPath = relative(resolvedBase, resolvedCase);
if (relPath.startsWith('..') || isAbsolute(relPath)) {
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case name');
src/web/routes/plan-routes.ts
+3 -5
View File
@@ -7,7 +7,6 @@
import { FastifyInstance } from 'fastify';
import { join, resolve, relative, isAbsolute } from 'node:path';
import { existsSync, rmSync } from 'node:fs';
import { homedir } from 'node:os';
import { Session } from '../../session.js';
import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js';
import { PlanOrchestrator, type PlanItem, type DetailedPlanResult } from '../../plan-orchestrator.js';
@@ -18,7 +17,7 @@ import {
PlanTaskUpdateSchema,
PlanTaskAddSchema,
} from '../schemas.js';
import { findSessionOrFail } from '../route-helpers.js';
import { findSessionOrFail, CASES_DIR } from '../route-helpers.js';
import type { SessionPort, EventPort, ConfigPort, InfraPort } from '../ports/index.js';
export function registerPlanRoutes(app: FastifyInstance, ctx: SessionPort & EventPort & ConfigPort & InfraPort): void {
@@ -228,11 +227,10 @@ NOW: Generate the implementation plan for the task above. Think step by step.`;
// Determine output directory for saving wizard results
let outputDir: string | undefined;
if (caseName) {
const casesDir = join(homedir(), 'codeman-cases');
const casePath = join(casesDir, caseName);
const casePath = join(CASES_DIR, caseName);
// Security: Path traversal protection - use relative path check
const resolvedCase = resolve(casePath);
const resolvedBase = resolve(casesDir);
const resolvedBase = resolve(CASES_DIR);
const relPath = relative(resolvedBase, resolvedCase);
if (!relPath.startsWith('..') && !isAbsolute(relPath) && existsSync(casePath)) {
outputDir = join(casePath, 'ralph-wizard');
src/web/routes/ralph-routes.ts
+7 -11
View File
@@ -8,21 +8,17 @@ import { FastifyInstance } from 'fastify';
import { join, dirname, resolve, relative, isAbsolute } from 'node:path';
import { existsSync, mkdirSync, writeFileSync } from 'node:fs';
import fs from 'node:fs/promises';
import { homedir } from 'node:os';
import { ApiErrorCode, createErrorResponse, getErrorMessage, type ApiResponse } from '../../types.js';
import { Session } from '../../session.js';
import { RespawnController } from '../../respawn-controller.js';
import { RalphConfigSchema, FixPlanImportSchema, RalphPromptWriteSchema, RalphLoopStartSchema } from '../schemas.js';
import { autoConfigureRalph } from '../route-helpers.js';
import { autoConfigureRalph, CASES_DIR, SETTINGS_PATH } from '../route-helpers.js';
import { writeHooksConfig } from '../../hooks-config.js';
import { generateClaudeMd } from '../../templates/claude-md.js';
import { getLifecycleLog } from '../../session-lifecycle-log.js';
import type { SessionPort, EventPort, RespawnPort, ConfigPort, InfraPort } from '../ports/index.js';
import { MAX_CONCURRENT_SESSIONS } from '../../config/map-limits.js';
const casesDir = join(homedir(), 'codeman-cases');
const settingsPath = join(homedir(), '.codeman', 'settings.json');
export function registerRalphRoutes(
app: FastifyInstance,
ctx: SessionPort & EventPort & RespawnPort & ConfigPort & InfraPort
@@ -305,11 +301,11 @@ export function registerRalphRoutes(
}
const { caseName, taskDescription, completionPhrase, maxIterations, enableRespawn, planItems } = rlResult.data;
const casePath = join(casesDir, caseName);
const casePath = join(CASES_DIR, caseName);
// Security: Path traversal protection
const rlResolvedPath = resolve(casePath);
const rlResolvedBase = resolve(casesDir);
const rlResolvedBase = resolve(CASES_DIR);
const rlRelPath = relative(rlResolvedBase, rlResolvedPath);
if (rlRelPath.startsWith('..') || isAbsolute(rlRelPath)) {
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case path');
@@ -435,7 +431,7 @@ export function registerRalphRoutes(
writeFileSync(promptPath, fullPrompt, 'utf-8');
// Register session
(ctx.sessions as Map<string, Session>).set(session.id, session);
ctx.addSession(session);
ctx.store.incrementSessionsCreated();
ctx.persistSessionState(session);
await ctx.setupSessionListeners(session);
@@ -489,14 +485,14 @@ export function registerRalphRoutes(
try {
let settings: Record<string, unknown> = {};
try {
settings = JSON.parse(await fs.readFile(settingsPath, 'utf-8'));
settings = JSON.parse(await fs.readFile(SETTINGS_PATH, 'utf-8'));
} catch {
/* ignore */
}
settings.lastUsedCase = caseName;
const dir = dirname(settingsPath);
const dir = dirname(SETTINGS_PATH);
if (!existsSync(dir)) mkdirSync(dir, { recursive: true });
fs.writeFile(settingsPath, JSON.stringify(settings, null, 2)).catch(() => {});
fs.writeFile(SETTINGS_PATH, JSON.stringify(settings, null, 2)).catch(() => {});
} catch {
/* non-critical */
}
src/web/routes/session-routes.ts
+9 -13
View File
@@ -8,7 +8,6 @@ import { FastifyInstance } from 'fastify';
import { join, dirname, resolve, relative, isAbsolute } from 'node:path';
import { existsSync, statSync, mkdirSync, writeFileSync } from 'node:fs';
import fs from 'node:fs/promises';
import { homedir } from 'node:os';
import {
ApiErrorCode,
createErrorResponse,
@@ -32,7 +31,8 @@ import {
QuickRunSchema,
QuickStartSchema,
} from '../schemas.js';
import { autoConfigureRalph } from '../route-helpers.js';
import { autoConfigureRalph, CASES_DIR, SETTINGS_PATH } from '../route-helpers.js';
import { AUTH_COOKIE_NAME } from '../middleware/auth.js';
import { writeHooksConfig, updateCaseEnvVars } from '../../hooks-config.js';
import { generateClaudeMd } from '../../templates/claude-md.js';
import { imageWatcher } from '../../image-watcher.js';
@@ -46,7 +46,6 @@ const MAX_INPUT_LENGTH = 64 * 1024;
const MAX_TERMINAL_COLS = 500;
const MAX_TERMINAL_ROWS = 200;
const MAX_SESSION_NAME_LENGTH = 128;
const AUTH_COOKIE_NAME = 'codeman_session';
// Pre-compiled regex for terminal buffer cleaning (avoids per-request compilation)
// eslint-disable-next-line no-control-regex
@@ -55,9 +54,6 @@ const CLAUDE_BANNER_PATTERN = /\x1b\[1mClaud/;
const CTRL_L_PATTERN = /\x0c/g;
const LEADING_WHITESPACE_PATTERN = /^[\s\r\n]+/;
const casesDir = join(homedir(), 'codeman-cases');
const settingsPath = join(homedir(), '.codeman', 'settings.json');
export function registerSessionRoutes(
app: FastifyInstance,
ctx: SessionPort & EventPort & ConfigPort & InfraPort
@@ -72,7 +68,7 @@ export function registerSessionRoutes(
// ========== Session Listing ==========
app.get('/api/sessions', async () => {
return Array.from(ctx.sessions.values()).map((s) => ctx.getSessionStateWithRespawn(s));
return ctx.getLightSessionsState();
});
// ========== Session Creation ==========
@@ -140,7 +136,7 @@ export function registerSessionRoutes(
openCodeConfig: mode === 'opencode' ? body.openCodeConfig : undefined,
});
(ctx.sessions as Map<string, Session>).set(session.id, session);
ctx.addSession(session);
ctx.store.incrementSessionsCreated();
ctx.persistSessionState(session);
await ctx.setupSessionListeners(session);
@@ -717,7 +713,7 @@ export function registerSessionRoutes(
}
const session = new Session({ workingDir: dir });
(ctx.sessions as Map<string, Session>).set(session.id, session);
ctx.addSession(session);
ctx.store.incrementSessionsCreated();
ctx.persistSessionState(session);
await ctx.setupSessionListeners(session);
@@ -770,11 +766,11 @@ export function registerSessionRoutes(
}
}
const casePath = join(casesDir, caseName);
const casePath = join(CASES_DIR, caseName);
// Security: Path traversal protection - use relative path check
const resolvedPath = resolve(casePath);
const resolvedBase = resolve(casesDir);
const resolvedBase = resolve(CASES_DIR);
const relPath = relative(resolvedBase, resolvedPath);
if (relPath.startsWith('..') || isAbsolute(relPath)) {
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Invalid case path');
@@ -832,7 +828,7 @@ export function registerSessionRoutes(
}
}
(ctx.sessions as Map<string, Session>).set(session.id, session);
ctx.addSession(session);
ctx.store.incrementSessionsCreated();
ctx.persistSessionState(session);
await ctx.setupSessionListeners(session);
@@ -870,7 +866,7 @@ export function registerSessionRoutes(
// Save lastUsedCase to settings for TUI/web sync
try {
const settingsFilePath = settingsPath;
const settingsFilePath = SETTINGS_PATH;
let settings: Record<string, unknown> = {};
try {
settings = JSON.parse(await fs.readFile(settingsFilePath, 'utf-8'));
src/web/routes/system-routes.ts
+10 -10
View File
@@ -22,7 +22,7 @@ import {
import { subagentWatcher } from '../../subagent-watcher.js';
import { imageWatcher } from '../../image-watcher.js';
import { getLifecycleLog } from '../../session-lifecycle-log.js';
import { findSessionOrFail, formatUptime } from '../route-helpers.js';
import { findSessionOrFail, formatUptime, SETTINGS_PATH } from '../route-helpers.js';
import type { SessionPort, EventPort, ConfigPort, InfraPort } from '../ports/index.js';
// Maximum screenshot upload size (10MB)
@@ -83,7 +83,6 @@ export function registerSystemRoutes(
app: FastifyInstance,
ctx: SessionPort & EventPort & ConfigPort & InfraPort
): void {
const settingsPath = join(homedir(), '.codeman', 'settings.json');
const windowStatesPath = join(homedir(), '.codeman', 'subagent-window-states.json');
const parentMapPath = join(homedir(), '.codeman', 'subagent-parents.json');
@@ -101,6 +100,7 @@ export function registerSystemRoutes(
return reply.code(404).send(createErrorResponse(ApiErrorCode.NOT_FOUND, 'Tunnel not running'));
}
try {
// eslint-disable-next-line @typescript-eslint/no-require-imports -- dynamic optional dependency
const QRCode = require('qrcode');
const svg: string = await QRCode.toString(url, { type: 'svg', margin: 2, width: 256 });
return { svg };
@@ -262,7 +262,7 @@ export function registerSystemRoutes(
app.get('/api/settings', async () => {
try {
const content = await fs.readFile(settingsPath, 'utf-8');
const content = await fs.readFile(SETTINGS_PATH, 'utf-8');
return JSON.parse(content);
} catch (err) {
if ((err as NodeJS.ErrnoException).code !== 'ENOENT') {
@@ -280,18 +280,18 @@ export function registerSystemRoutes(
const settings = settingsResult.data as Record<string, unknown>;
try {
const dir = dirname(settingsPath);
const dir = dirname(SETTINGS_PATH);
if (!existsSync(dir)) {
mkdirSync(dir, { recursive: true });
}
let existing: Record<string, unknown> = {};
try {
existing = JSON.parse(await fs.readFile(settingsPath, 'utf-8'));
existing = JSON.parse(await fs.readFile(SETTINGS_PATH, 'utf-8'));
} catch {
/* ignore */
}
const merged = { ...existing, ...settings };
await fs.writeFile(settingsPath, JSON.stringify(merged, null, 2));
await fs.writeFile(SETTINGS_PATH, JSON.stringify(merged, null, 2));
// Handle subagent tracking toggle dynamically
const subagentEnabled = settings.subagentTrackingEnabled ?? true;
@@ -345,7 +345,7 @@ export function registerSystemRoutes(
app.get('/api/execution/model-config', async () => {
try {
const content = await fs.readFile(settingsPath, 'utf-8');
const content = await fs.readFile(SETTINGS_PATH, 'utf-8');
const settings = JSON.parse(content);
return { success: true, data: settings.modelConfig || {} };
} catch (err) {
@@ -366,7 +366,7 @@ export function registerSystemRoutes(
try {
let existingSettings: Record<string, unknown> = {};
try {
const content = await fs.readFile(settingsPath, 'utf-8');
const content = await fs.readFile(SETTINGS_PATH, 'utf-8');
existingSettings = JSON.parse(content);
} catch {
// File doesn't exist yet, start fresh
@@ -374,11 +374,11 @@ export function registerSystemRoutes(
existingSettings.modelConfig = modelConfig;
const dir = dirname(settingsPath);
const dir = dirname(SETTINGS_PATH);
if (!existsSync(dir)) {
mkdirSync(dir, { recursive: true });
}
await fs.writeFile(settingsPath, JSON.stringify(existingSettings, null, 2));
await fs.writeFile(SETTINGS_PATH, JSON.stringify(existingSettings, null, 2));
return { success: true };
} catch (err) {
src/web/server.ts
+4
View File
@@ -440,6 +440,9 @@ export class WebServer extends EventEmitter {
return {
// SessionPort
sessions: this.sessions as ReadonlyMap<string, Session>,
addSession: (session: Session) => {
this.sessions.set(session.id, session);
},
cleanupSession: this.cleanupSession.bind(this),
setupSessionListeners: this.setupSessionListeners.bind(this),
persistSessionState: this.persistSessionState.bind(this),
@@ -469,6 +472,7 @@ export class WebServer extends EventEmitter {
getClaudeModeConfig: this.getClaudeModeConfig.bind(this),
getDefaultClaudeMdPath: this.getDefaultClaudeMdPath.bind(this),
getLightState: this.getLightState.bind(this),
getLightSessionsState: this.getLightSessionsState.bind(this),
startTranscriptWatcher: this.startTranscriptWatcher.bind(this),
stopTranscriptWatcher: this.stopTranscriptWatcher.bind(this),
// InfraPort