-
Notifications
You must be signed in to change notification settings - Fork 232
fix: remove unsafe any usage #445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
47ac50a
6cf3ecd
386f349
1f1f5c7
e416f07
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,8 @@ import type { | |
| DeploymentType, | ||
| HealthStatus, | ||
| ServerlessEvent, | ||
| ServerlessResponse | ||
| ServerlessResponse, | ||
| RawExecutionContext | ||
| } from '../types/agent.js'; | ||
| import { ReasonerRegistry } from './ReasonerRegistry.js'; | ||
| import { SkillRegistry } from './SkillRegistry.js'; | ||
|
|
@@ -43,16 +44,43 @@ import { | |
| type ExecutionLogger | ||
| } from '../observability/ExecutionLogger.js'; | ||
| import { LocalVerifier } from '../verification/LocalVerifier.js'; | ||
| import type { Request, Response } from 'express'; | ||
| import type { ParamsDictionary } from 'express-serve-static-core'; | ||
| import { | ||
| installStdioLogCapture, | ||
| ProcessLogRing, | ||
| registerAgentfieldLogsRoute | ||
| } from './processLogs.js'; | ||
|
|
||
| interface WildcardParams extends ParamsDictionary { | ||
| 0: string; | ||
| } | ||
| class TargetNotFoundError extends Error {} | ||
|
|
||
| const harnessRunners = new WeakMap<object, HarnessRunner>(); | ||
|
|
||
|
|
||
|
|
||
| function normalizeExecutionContext( | ||
| ctx: RawExecutionContext | ||
| ): Partial<ExecutionMetadata> { | ||
| return { | ||
| executionId: ctx.executionId ?? ctx.execution_id, | ||
| runId: ctx.runId ?? ctx.run_id, | ||
| workflowId: ctx.workflowId ?? ctx.workflow_id, | ||
| parentExecutionId: ctx.parentExecutionId ?? ctx.parent_execution_id, | ||
| sessionId: ctx.sessionId ?? ctx.session_id, | ||
| actorId: ctx.actorId ?? ctx.actor_id, | ||
| callerDid: ctx.callerDid ?? ctx.caller_did, | ||
| targetDid: ctx.targetDid ?? ctx.target_did, | ||
| agentNodeDid: ctx.agentNodeDid ?? ctx.agent_node_did, | ||
|
|
||
| // ✅ ADD THESE | ||
| rootWorkflowId: (ctx as any).rootWorkflowId ?? (ctx as any).root_workflow_id, | ||
| reasonerId: (ctx as any).reasonerId ?? (ctx as any).reasoner_id | ||
| }; | ||
| } | ||
|
|
||
| export class Agent { | ||
| readonly config: AgentConfig; | ||
| readonly app: express.Express; | ||
|
|
@@ -739,10 +767,10 @@ export class Agent { | |
| }); | ||
| } | ||
|
|
||
| this.app.post('/api/v1/reasoners/*', (req, res) => this.executeReasoner(req, res, (req.params as any)[0])); | ||
| this.app.post('/api/v1/reasoners/*', (req: Request<WildcardParams>, res: Response) => this.executeReasoner(req, res, req.params[0])); | ||
| this.app.post('/reasoners/:name', (req, res) => this.executeReasoner(req, res, req.params.name)); | ||
|
|
||
| this.app.post('/api/v1/skills/*', (req, res) => this.executeSkill(req, res, (req.params as any)[0])); | ||
| this.app.post('/api/v1/skills/*', (req: Request<WildcardParams>, res: Response) => this.executeSkill(req, res, req.params[0])); | ||
| this.app.post('/skills/:name', (req, res) => this.executeSkill(req, res, req.params.name)); | ||
|
|
||
| // Serverless-friendly execute endpoint that accepts { target, input } or { reasoner, input } | ||
|
|
@@ -875,7 +903,7 @@ export class Agent { | |
|
|
||
| private handleHttpRequest(req: http.IncomingMessage | express.Request, res: http.ServerResponse | express.Response) { | ||
| const handler = this.app as unknown as (req: http.IncomingMessage, res: http.ServerResponse) => void; | ||
| return handler(req as any, res as any); | ||
| return handler(req as http.IncomingMessage, res as http.ServerResponse); | ||
| } | ||
|
|
||
| private async handleServerlessEvent(event: ServerlessEvent): Promise<ServerlessResponse> { | ||
|
|
@@ -890,7 +918,7 @@ export class Agent { | |
| }; | ||
| } | ||
|
|
||
| const body = this.normalizeEventBody(event); | ||
| const body = event?.body !== undefined ? this.parseBody(event.body): event; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changes behavior, not just types. The old code called Serverless callers that send |
||
| const invocation = this.extractInvocationDetails({ | ||
| path, | ||
| query: event?.queryStringParameters, | ||
|
|
@@ -938,48 +966,23 @@ export class Agent { | |
| } | ||
|
|
||
| private normalizeEventBody(event: ServerlessEvent) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method is now dead code — nothing calls it after the change on line 921. It also has completely different logic from the original (returns Either restore this as the call target in |
||
| const parsed = this.parseBody((event as any)?.body); | ||
| if (parsed && typeof parsed === 'object' && event?.input !== undefined && (parsed as any).input === undefined) { | ||
| return { ...(parsed as Record<string, any>), input: event.input }; | ||
| } | ||
| if ((parsed === undefined || parsed === null) && event?.input !== undefined) { | ||
| return { input: event.input }; | ||
| interface ParsedBody { | ||
| input?: unknown; | ||
| data?: unknown; | ||
| [key: string]: unknown; | ||
| } | ||
|
|
||
| const parsed = this.parseBody(event?.body) as ParsedBody | undefined; | ||
|
|
||
| if (parsed?.input !== undefined) return parsed.input; | ||
| if (parsed?.data !== undefined) return parsed.data; | ||
|
|
||
| return parsed; | ||
| } | ||
|
|
||
| private mergeExecutionContext(event: ServerlessEvent): Partial<ExecutionMetadata> { | ||
| const ctx = (event?.executionContext ?? (event as any)?.execution_context) as Partial< | ||
| ExecutionMetadata & { | ||
| execution_id?: string; | ||
| run_id?: string; | ||
| workflow_id?: string; | ||
| root_workflow_id?: string; | ||
| parent_execution_id?: string; | ||
| reasoner_id?: string; | ||
| session_id?: string; | ||
| actor_id?: string; | ||
| caller_did?: string; | ||
| target_did?: string; | ||
| agent_node_did?: string; | ||
| } | ||
| >; | ||
|
|
||
| if (!ctx) return {}; | ||
|
|
||
| return { | ||
| executionId: (ctx as any).executionId ?? ctx.execution_id ?? ctx.executionId, | ||
| runId: ctx.runId ?? (ctx as any).run_id, | ||
| workflowId: ctx.workflowId ?? (ctx as any).workflow_id, | ||
| rootWorkflowId: ctx.rootWorkflowId ?? (ctx as any).root_workflow_id, | ||
| parentExecutionId: ctx.parentExecutionId ?? (ctx as any).parent_execution_id, | ||
| reasonerId: ctx.reasonerId ?? (ctx as any).reasoner_id, | ||
| sessionId: ctx.sessionId ?? (ctx as any).session_id, | ||
| actorId: ctx.actorId ?? (ctx as any).actor_id, | ||
| callerDid: (ctx as any).callerDid ?? (ctx as any).caller_did, | ||
| targetDid: (ctx as any).targetDid ?? (ctx as any).target_did, | ||
| agentNodeDid: (ctx as any).agentNodeDid ?? (ctx as any).agent_node_did | ||
| }; | ||
| const rawCtx = event?.executionContext ?? event?.execution_context; | ||
| return rawCtx ? normalizeExecutionContext(rawCtx) : {}; | ||
| } | ||
|
|
||
| private extractInvocationDetails(params: { | ||
|
|
@@ -1064,12 +1067,15 @@ export class Agent { | |
|
|
||
| if (parsed && typeof parsed === 'object') { | ||
| const { target, reasoner, skill, type, targetType, ...rest } = parsed as Record<string, any>; | ||
| if ((parsed as any).input !== undefined) { | ||
| return (parsed as any).input; | ||
| } | ||
| if ((parsed as any).data !== undefined) { | ||
| return (parsed as any).data; | ||
| interface ParsedBody { | ||
| input?: any; | ||
| data?: any; | ||
| [key: string]: any; | ||
| } | ||
|
|
||
| const parsedBody = parsed as ParsedBody; | ||
| if (parsedBody.input !== undefined) return parsedBody.input; | ||
| if (parsedBody.data !== undefined) return parsedBody.data; | ||
| if (Object.keys(rest).length === 0) { | ||
| return {}; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,11 +3,33 @@ import os from 'node:os'; | |
|
|
||
| export class RateLimitError extends Error { | ||
| retryAfter?: number; | ||
|
|
||
| constructor(message: string, retryAfter?: number) { | ||
| status?: number; | ||
| statusCode?: number; | ||
| response?: { | ||
| status?: number; | ||
| statusCode?: number; | ||
| status_code?: number; | ||
| headers?: Record<string, string>; | ||
| }; | ||
|
|
||
| constructor( | ||
| message: string, | ||
| retryAfter?: number, | ||
| status?: number, | ||
| statusCode?: number, | ||
| response?: { | ||
| status?: number; | ||
| statusCode?: number; | ||
| status_code?: number; | ||
| headers?: Record<string, string>; | ||
| } | ||
| ) { | ||
| super(message); | ||
| this.name = 'RateLimitError'; | ||
| this.retryAfter = retryAfter; | ||
| this.status = status; | ||
| this.statusCode = statusCode; | ||
| this.response = response; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -57,26 +79,38 @@ export class StatelessRateLimiter { | |
|
|
||
| protected _isRateLimitError(error: unknown): boolean { | ||
| if (!error) return false; | ||
| const err = error as any; | ||
|
|
||
| const className = err?.constructor?.name; | ||
| if (className && className.includes('RateLimitError')) { | ||
| if (error instanceof RateLimitError) { | ||
| return true; | ||
| } | ||
|
|
||
| const response = err?.response; | ||
| if ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: the indentation is inconsistent through this block — mixes 2-space and 4-space, and the |
||
| typeof error === 'object' && | ||
| error !== null && | ||
| typeof (error as { constructor?: { name?: string } }).constructor?.name === 'string' && | ||
| (error as { constructor: { name: string } }).constructor.name.includes('RateLimitError') | ||
| ) { | ||
| return true; | ||
| } | ||
|
|
||
| if (isRateLimitError(error)) { | ||
| const statusCandidates = [ | ||
| err?.status, | ||
| err?.statusCode, | ||
| response?.status, | ||
| response?.statusCode, | ||
| response?.status_code | ||
| error.status, | ||
| error.statusCode, | ||
| error.response?.status, | ||
| error.response?.statusCode, | ||
| error.response?.status_code, | ||
| ]; | ||
| if (statusCandidates.some((code: any) => code === 429 || code === 503)) { | ||
| if (statusCandidates.some((code) => code === 429 || code === 503)) { | ||
| return true; | ||
| } | ||
|
|
||
| const message = String(err?.message ?? err ?? '').toLowerCase(); | ||
| if (error.retryAfter !== undefined) { | ||
| return true; | ||
| } | ||
| } | ||
| const err = toError(error); | ||
| const message = err.message.toLowerCase(); | ||
| const rateLimitKeywords = [ | ||
| 'rate limit', | ||
| 'rate-limit', | ||
|
|
@@ -98,26 +132,40 @@ export class StatelessRateLimiter { | |
|
|
||
| protected _extractRetryAfter(error: unknown): number | undefined { | ||
| if (!error) return undefined; | ||
| const err = error as any; | ||
| if (error instanceof RateLimitError) { | ||
| if (error.retryAfter) { | ||
| return error.retryAfter; | ||
| } | ||
| } | ||
|
|
||
| const headers = err?.response?.headers ?? err?.response?.Headers ?? err?.response?.header; | ||
| if (isRateLimitError(error)) { | ||
| const headers = error.response?.headers; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old code had |
||
| if (headers && typeof headers === 'object') { | ||
| const retryAfterKey = Object.keys(headers).find((k) => k.toLowerCase() === 'retry-after'); | ||
| if (retryAfterKey) { | ||
| const value = Array.isArray(headers[retryAfterKey]) ? headers[retryAfterKey][0] : headers[retryAfterKey]; | ||
| const parsed = parseFloat(value); | ||
| if (!Number.isNaN(parsed)) { | ||
| return parsed; | ||
| if (value !== undefined) { | ||
| const parsed = parseFloat(String(value)); | ||
| if (!Number.isNaN(parsed)) { | ||
| return parsed; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const retryAfter = err?.retryAfter ?? err?.retry_after; | ||
| const parsed = parseFloat(retryAfter); | ||
| if (!Number.isNaN(parsed)) { | ||
| return parsed; | ||
| if (error.retryAfter !== undefined) { | ||
| const parsed = parseFloat(String(error.retryAfter)); | ||
| if (!Number.isNaN(parsed)) { | ||
| return parsed; | ||
| } | ||
| } | ||
| } | ||
| const err = error as { retryAfter?: string | number }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old code also checked |
||
| if (err.retryAfter !== undefined) { | ||
| const parsed = parseFloat(String(err.retryAfter)); | ||
| if (!Number.isNaN(parsed)) { | ||
| return parsed; | ||
| } | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
|
|
@@ -219,3 +267,24 @@ export class StatelessRateLimiter { | |
| ); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| function toError(error: unknown): Error { | ||
| if (error instanceof Error) return error; | ||
| return new Error(String(error)); | ||
| } | ||
|
|
||
|
|
||
| function isRateLimitError(error: unknown): error is RateLimitError { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This type guard is pretty loose — any object with Might be safer to require that the status is specifically 429 or 503, or check for rate-limit-specific property combinations. |
||
| return ( | ||
| typeof error === 'object' && | ||
| error !== null && | ||
| ('name' in error || 'message' in error) && | ||
| ( | ||
| 'status' in error || | ||
| 'statusCode' in error || | ||
| 'response' in error || | ||
| 'retryAfter' in error | ||
| ) | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover dev comment — should be removed before merging.
Also, the reason you need
as anyon the next two lines is thatRawExecutionContextis missingrootWorkflowId/root_workflow_idandreasonerId/reasoner_id. If you add those to the interface intypes/agent.ts, these casts go away and the type is actually complete.