feat: add OpenClaw loopback HTTP fast path#793
feat: add OpenClaw loopback HTTP fast path#793Nikhil (shadowfax92) wants to merge 6 commits intodevfrom
Conversation
|
Claude (@claude) review |
|
Greptile (@greptileai) review |
|
Claude finished Nikhil (@shadowfax92)'s task in 2m 14s —— View job Code Review: feat/add-openclaw-ws
Overall this is a clean, well-scoped refactor. The separation of "CLI for cold-path mutations, HTTP for hot-path reads" is a sound design, the error-handling helpers are thorough, and test coverage is solid. A few issues worth addressing before merge: Bug:
|
Greptile SummaryThis PR replaces token-based gateway auth with a no-auth loopback fast path: it introduces
Confidence Score: 4/5Two P1 defects need addressing before merge: unvalidated NaN limit forwarded to gateway, and silent auth-mode migration failure that can leave the service permanently broken. The overall architecture and new HTTP client are clean, but the unvalidated limit=NaN forwarded upstream and the swallowed migration failure that can silently break all control-plane calls are present defects on the changed code paths. openclaw.ts (NaN limit validation) and openclaw-service.ts (ensureAuthModeNone silent failure handling) Important Files Changed
Sequence DiagramsequenceDiagram
participant Route as GET /claw/session/:key/history
participant Svc as OpenClawService
participant HTTP as OpenClawHttpClient
participant GW as OpenClaw Gateway (loopback)
Route->>Svc: getSessionHistory(key, {limit, cursor, signal})
Svc->>HTTP: getSessionHistory(key, input)
HTTP->>GW: GET /sessions/{encoded_key}/history
GW-->>HTTP: 200 OpenClawSessionHistory or 404
alt 404
HTTP-->>Svc: throw OpenClawSessionNotFoundError
Route-->>Route: return 404
else 2xx
HTTP-->>Svc: OpenClawSessionHistory
Route-->>Route: return 200 JSON
end
Note over Svc,GW: Boot/restart path
Svc->>Svc: ensureAuthModeNone()
Svc->>GW: setConfig gateway.auth.mode none
Svc->>HTTP: probe() GET /v1/models
HTTP->>GW: GET /v1/models
GW-->>HTTP: 200/204
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const cursor = c.req.query('cursor') | ||
| const limit = | ||
| limitRaw !== undefined ? Number.parseInt(limitRaw, 10) : undefined | ||
|
|
||
| try { |
There was a problem hiding this comment.
NaN forwarded to gateway when limit is non-numeric
Number.parseInt('abc', 10) returns NaN, and since NaN !== undefined, the check in buildHistoryPath passes the value through, producing ?limit=NaN in the upstream URL. Depending on gateway behaviour this could result in a 400, unintended unbounded reads, or the param being silently ignored — none of which match the caller's intent.
Add a guard after parsing:
| const cursor = c.req.query('cursor') | |
| const limit = | |
| limitRaw !== undefined ? Number.parseInt(limitRaw, 10) : undefined | |
| try { | |
| const limit = | |
| limitRaw !== undefined ? Number.parseInt(limitRaw, 10) : undefined | |
| if (limit !== undefined && !Number.isFinite(limit)) { | |
| return c.json({ error: 'invalid_limit' }, 400) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/api/routes/openclaw.ts
Line: 351-355
Comment:
**`NaN` forwarded to gateway when `limit` is non-numeric**
`Number.parseInt('abc', 10)` returns `NaN`, and since `NaN !== undefined`, the check in `buildHistoryPath` passes the value through, producing `?limit=NaN` in the upstream URL. Depending on gateway behaviour this could result in a 400, unintended unbounded reads, or the param being silently ignored — none of which match the caller's intent.
Add a guard after parsing:
```suggestion
const limit =
limitRaw !== undefined ? Number.parseInt(limitRaw, 10) : undefined
if (limit !== undefined && !Number.isFinite(limit)) {
return c.json({ error: 'invalid_limit' }, 400)
}
```
How can I resolve this? If you propose a fix, please make it concise.
✅ Tests passed — 812/815
|
1 participant
Summary
gateway.auth.mode = "none"on loopback and remove token-based admin reads from service hot pathsOpenClawHttpClientfor/v1/models,/tools/invoke, and JSON session history reads on/sessions/:key/historyGET /claw/session/:key/historyas a JSON-only route and keep session history fetches on loopback HTTPagents listafteragents add; the service now refetches the created agent over HTTPDesign
OpenClaw still uses
podman execfor cold-path mutations like onboarding, config writes, and agent create/delete, but all hot-path admin reads now go directly to the loopback gateway over HTTP with no auth header. The service enforcesgateway.auth.mode = none, migrates existing configs once on start/restart, and keeps session history as a simple pull endpoint instead of a streamed route.Test Plan
bun test tests/api/services/openclaw/openclaw-http-client.test.tsbun test tests/api/services/openclaw/openclaw-service.test.tsbun test tests/api/routes/openclaw.test.tsbun run typecheckcd apps/server && bun run test