fix: stabilize v0.5 routes and restore TypeScript build#10
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| if (temperature !== undefined && (temperature < 0 || temperature > 2)) { | ||
| return { ok: false, error: 'temperature must be between 0 and 2' }; | ||
| } | ||
| if (maxTokens !== undefined && (maxTokens < 1 || maxTokens > 32768)) { |
There was a problem hiding this comment.
WARNING: Hard max_tokens cap of 32768 may reject valid requests for large-context models
Many modern LLMs (Llama 3, Qwen 2.5, Mistral Large, etc.) support context windows of 128k tokens or more. Capping max_tokens at 32 768 means the endpoint will return a 400 for any client that passes a higher value based on the model's actual capacity — silently breaking a previously-valid use-case. LM Studio and the underlying runtime will already enforce their own context limits, so this pre-validation adds risk without meaningful safety gain. Consider either raising the ceiling to match a realistic upper bound (e.g. 131072) or removing the upper-bound check entirely.
| @@ -0,0 +1 @@ | |||
| export const APP_VERSION = '0.5.0'; | |||
There was a problem hiding this comment.
SUGGESTION: APP_VERSION is defined independently in both packages/server/src/config/version.ts and packages/dashboard/src/config/version.ts. These will silently diverge on the next version bump if only one file is updated. A single source of truth is safer: on the server, process.env.npm_package_version is available at runtime (or import from package.json with assert { type: 'json' }); on the dashboard, a Vite define in vite.config.ts can inject the root package.json version at build time.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Other Observations (not in diff)The PR description notes that Files Reviewed (13 files)
Fix these issues in Kilo Cloud Reviewed by claude-4.6-sonnet-20260217 · 183,504 tokens |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request bumps the project version to 0.5.0, adds workspace-wide TypeScript checks, and replaces several placeholder environment variable checks with robust validation and logic. It enhances security by binding the server to localhost by default, adding a CORS origin whitelist, and warning when exposed externally. Additionally, a dedicated benchmarkState service is introduced to manage benchmark results. Feedback on the changes includes wrapping the streaming chat endpoint in a try/catch block to handle potential promise rejections, rejecting CORS requests cleanly with callback(null, false) instead of throwing an error, and using a loose inequality check for profile.firstTokenMs to guard against undefined values.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| await orchestrator.sendMessageStream( | ||
| req.params.id, | ||
| content, | ||
| // TODO: Move to env: (token) | ||
| const (token) = process.env.(TOKEN) || ''; | ||
| parsed.value.content, | ||
| (token) => { | ||
| writeSse(res, { type: 'token', content: token }); | ||
| }, | ||
| (fullResponse) => { | ||
| res.write(`data: ${JSON.stringify({ type: 'done', content: fullResponse })}\n\n`); | ||
| writeSse(res, { type: 'done', content: fullResponse }); | ||
| res.end(); | ||
| }, | ||
| (err) => { | ||
| res.write(`data: ${JSON.stringify({ type: 'error', error: String(err) })}\n\n`); | ||
| writeSse(res, { type: 'error', error: String(err) }); | ||
| res.end(); | ||
| } | ||
| ); |
There was a problem hiding this comment.
The asynchronous call await orchestrator.sendMessageStream(...) is not wrapped in a try/catch block. If sendMessageStream rejects (for example, if the session ID is invalid or a connection error occurs before streaming starts), it results in an unhandled promise rejection. Since headers have already been written, this can cause the Node.js process to crash or leave the connection hanging.
Wrap the call in a try/catch block to handle any synchronous or asynchronous initialization errors gracefully.
| await orchestrator.sendMessageStream( | |
| req.params.id, | |
| content, | |
| // TODO: Move to env: (token) | |
| const (token) = process.env.(TOKEN) || ''; | |
| parsed.value.content, | |
| (token) => { | |
| writeSse(res, { type: 'token', content: token }); | |
| }, | |
| (fullResponse) => { | |
| res.write(`data: ${JSON.stringify({ type: 'done', content: fullResponse })}\n\n`); | |
| writeSse(res, { type: 'done', content: fullResponse }); | |
| res.end(); | |
| }, | |
| (err) => { | |
| res.write(`data: ${JSON.stringify({ type: 'error', error: String(err) })}\n\n`); | |
| writeSse(res, { type: 'error', error: String(err) }); | |
| res.end(); | |
| } | |
| ); | |
| try { | |
| await orchestrator.sendMessageStream( | |
| req.params.id, | |
| parsed.value.content, | |
| (token) => { | |
| writeSse(res, { type: 'token', content: token }); | |
| }, | |
| (fullResponse) => { | |
| writeSse(res, { type: 'done', content: fullResponse }); | |
| res.end(); | |
| }, | |
| (err) => { | |
| writeSse(res, { type: 'error', error: String(err) }); | |
| res.end(); | |
| } | |
| ); | |
| } catch (err) { | |
| writeSse(res, { type: 'error', error: String(err) }); | |
| res.end(); | |
| } |
| const corsOptions: CorsOptions = { | ||
| origin(origin, callback) { | ||
| if (!origin || LOCALHOST_ORIGINS.has(origin)) { | ||
| callback(null, true); | ||
| return; | ||
| } | ||
| callback(new Error(`CORS origin not allowed: ${origin}`)); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
In Express cors middleware, passing an Error object to the callback (callback(new Error(...))) forwards the error to the Express global error handler, which typically results in a 500 Internal Server Error and logs a noisy stack trace.
To reject a CORS request cleanly without throwing a server-side exception, pass false as the second argument: callback(null, false). This blocks the CORS request at the HTTP header level (by not sending the Access-Control-Allow-Origin header) without disrupting the server's execution flow.
| const corsOptions: CorsOptions = { | |
| origin(origin, callback) { | |
| if (!origin || LOCALHOST_ORIGINS.has(origin)) { | |
| callback(null, true); | |
| return; | |
| } | |
| callback(new Error(`CORS origin not allowed: ${origin}`)); | |
| }, | |
| }; | |
| const corsOptions: CorsOptions = { | |
| origin(origin, callback) { | |
| if (!origin || LOCALHOST_ORIGINS.has(origin)) { | |
| callback(null, true); | |
| return; | |
| } | |
| callback(null, false); | |
| }, | |
| }; |
| function estimateFirstTokenMs(profile: ModelProfile): number { | ||
| if (profile.firstTokenMs !== null) return profile.firstTokenMs; | ||
| const tokS = profile.tokSGpu || profile.tokSCpu || 0; | ||
| return tokS > 0 ? Math.round(1000 / tokS) : Infinity; | ||
| } |
There was a problem hiding this comment.
The strict inequality check profile.firstTokenMs !== null does not guard against undefined. If profile.firstTokenMs is optional or missing (resulting in undefined), the function returns undefined instead of falling back to the calculation. This violates the number return type at runtime and can lead to NaN values when performing math operations like Math.round(firstToken).
Using loose inequality profile.firstTokenMs != null (or checking both null and undefined) ensures a safe fallback.
| function estimateFirstTokenMs(profile: ModelProfile): number { | |
| if (profile.firstTokenMs !== null) return profile.firstTokenMs; | |
| const tokS = profile.tokSGpu || profile.tokSCpu || 0; | |
| return tokS > 0 ? Math.round(1000 / tokS) : Infinity; | |
| } | |
| function estimateFirstTokenMs(profile: ModelProfile): number { | |
| if (profile.firstTokenMs != null) return profile.firstTokenMs; | |
| const tokS = profile.tokSGpu || profile.tokSCpu || 0; | |
| return tokS > 0 ? Math.round(1000 / tokS) : Infinity; | |
| } |
1 participant
Summary
Stabilizes the v0.5 local-first baseline by removing invalid generated TypeScript fragments, validating risky route inputs, aligning version metadata, and making local development safer by default.
Key fixes
/sessions/:id/message/stream,/openclaw/usage,/lmstudio/chat, and/lmstudio/chat/stream.globalThis as anywith a typed in-memorybenchmarkStateservice.3001.127.0.0.1with a warning for non-local binds.0.5.0across package metadata, health response, startup banner, and dashboard badge.typecheckscripts and updated local development docs.Files changed
packages/server/src/api/routes.tspackages/server/src/index.tspackages/server/src/services/benchmark.tspackages/server/src/services/route-advisor.tspackages/server/src/services/benchmark-state.tspackages/server/src/config/version.tspackages/dashboard/src/components/Dashboard.tsxpackages/dashboard/src/config/version.tspackage.json,package-lock.jsonpackages/server/package.json,packages/dashboard/package.jsonREADME.mdCommands run
cd packages/server && npx tsc --noEmitcd packages/dashboard && npx tsc --noEmitnpm run typechecknpm run buildnpm run dev:serversmoke withGET http://127.0.0.1:3001/api/healthws://127.0.0.1:3001/wsnpm run dev:dashboardsmoke againsthttp://localhost:3000git diff --checkconst const,CONST_,process.env.(,const if, andglobalThis as anyKnown limitations
anyusages remain elsewhere in server/dashboard code; this PR removes the worst touchedglobalThis as anypath and avoids adding new productionany.lmsCLI integration; this PR does not expand command execution behavior.Follow-up backlog
anywith typed service boundaries.