fix: issues with build & broken actions #1
Conversation
WalkthroughAction handlers were migrated from returning booleans/Content/void to returning structured ActionResult objects; error paths were changed from throws to returned ActionResult; LLM extraction fallbacks and some relaxed typings were added. Package metadata was scoped/versioned, registry entries removed, Changes
Notes: "src/actions/* (see list)" above includes all action files modified in the diff (e.g., Sequence Diagram(s)sequenceDiagram
autonumber
participant U as Caller
participant A as Action Handler
participant L as LLM Extractor
participant C as CLOB API
participant CB as Callback
U->>A: invoke handler(context, runtime, message, state, cb)
A->>L: attempt param extraction (LLM)
alt LLM success
L-->>A: params
else LLM fail
L-->>A: failure → attempt regex fallback
end
alt no valid params
A->>CB: cb({ text, data: { actionName, error, timestamp } })
A-->>U: return ActionResult (error)
else params valid
A->>C: perform API call(s)
alt API error
A->>CB: cb({ text, data: { actionName, error, timestamp } })
A-->>U: return ActionResult (error)
else success
A->>CB: cb({ text, data: { actionName, ...result, timestamp } })
A-->>U: return ActionResult (success)
end
end
sequenceDiagram
autonumber
participant U as Caller
participant H as handleAuthenticationAction
participant K as API Key Service
participant R as Runtime Settings
participant CB as Callback
U->>H: invoke handler(..., cb)
H->>K: createOrDeriveApiKey()
alt failure/invalid response
H->>CB: cb({ text, data: { actionName, error, timestamp } })
H-->>U: return ActionResult (error)
else success
H->>R: set CLOB_API_KEY / CLOB_API_SECRET / CLOB_API_PASSPHRASE
H->>CB: cb({ text, data: { actionName, derivedCreds, timestamp } })
H-->>U: return ActionResult (success)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
src/actions/revokeApiKey.ts (3)
195-201: Use ESM import for crypto;requirebreaks under ESM/TS configsSwitch to
node:cryptoimport and callcreateHmacdirectly.+import { createHmac } from 'node:crypto'; @@ - const crypto = require('crypto'); - const message_to_sign = timestamp + method + requestPath + JSON.stringify({ key: apiKeyId }); - const signature = crypto - .createHmac('sha256', apiSecret) - .update(message_to_sign) - .digest('base64'); + const message_to_sign = + timestamp + method + requestPath + JSON.stringify({ key: apiKeyId }); + const signature = createHmac('sha256', apiSecret) + .update(message_to_sign) + .digest('base64');
223-224: Don’t unconditionally parse JSON on 204/empty responsesIf the API returns 204 or empty body,
response.json()throws.- const deleteResult = await deleteResponse.json(); + // Optional: parse JSON only when present + const contentType = deleteResponse.headers.get('content-type') || ''; + if (contentType.includes('application/json')) { + await deleteResponse.json(); + }
189-201: Use milliseconds for POLY_TIMESTAMP and ensure L2 headers are correctPolymarket CLOB expects POLY_TIMESTAMP in milliseconds and requires POLY_ADDRESS, POLY_SIGNATURE, POLY_TIMESTAMP, POLY_API_KEY, POLY_PASSPHRASE for private endpoints (including DELETE /auth/api-key). In src/actions/revokeApiKey.ts (lines 189–201) replace Math.floor(Date.now() / 1000).toString() with Date.now().toString() and confirm the request sets the listed POLY_* headers.
src/actions/getSamplingMarkets.ts (1)
97-109: Callback payload shape diverges from Content used elsewhereOther actions use Content with actions/data. Here a nested content object is passed, which may break downstream consumers that expect actions/data. Align shapes.
- if (callback) { - await callback({ - text: responseMessage, - content: { - action: 'POLYMARKET_SAMPLING_MARKETS_RETRIEVED', - markets: markets, - count: totalCount, - next_cursor: nextCursor, - timestamp: new Date().toISOString(), - }, - }); - } + if (callback) { + await callback({ + text: responseMessage, + actions: ['POLYMARKET_GET_SAMPLING_MARKETS'], + data: { + markets, + count: totalCount, + next_cursor: nextCursor, + timestamp: new Date().toISOString(), + }, + }); + }- if (callback) { - await callback({ - text: errorMessage, - content: { - action: 'POLYMARKET_SAMPLING_MARKETS_ERROR', - error: error instanceof Error ? error.message : 'Unknown error', - timestamp: new Date().toISOString(), - }, - }); - } + if (callback) { + await callback({ + text: errorMessage, + actions: ['POLYMARKET_GET_SAMPLING_MARKETS'], + data: { + error: error instanceof Error ? error.message : 'Unknown error', + timestamp: new Date().toISOString(), + }, + }); + }Also applies to: 121-130
src/actions/getOpenMarkets.ts (1)
159-171: Unify callback Content shape (use actions/data)Same issue as sampling markets: nested content differs from the Content shape used elsewhere. Align to avoid consumer breakage.
- if (callback) { - await callback({ - text: responseMessage, - content: { - action: 'open_markets_retrieved', - markets: finalMarketsToReturn, - count: finalMarketsToReturn.length, - total_open: openMarkets.length, - total_fetched: allMarkets.length, - filters: params, - timestamp: new Date().toISOString(), - }, - }); - } + if (callback) { + await callback({ + text: responseMessage, + actions: ['POLYMARKET_GET_OPEN_MARKETS'], + data: { + markets: finalMarketsToReturn, + count: finalMarketsToReturn.length, + total_open: openMarkets.length, + total_fetched: allMarkets.length, + filters: params, + timestamp: new Date().toISOString(), + }, + }); + }- if (callback) { - await callback({ - text: errorMessage, - content: { - action: 'open_markets_error', - error: error instanceof Error ? error.message : 'Unknown error', - timestamp: new Date().toISOString(), - }, - }); - } + if (callback) { + await callback({ + text: errorMessage, + actions: ['POLYMARKET_GET_OPEN_MARKETS'], + data: { + error: error instanceof Error ? error.message : 'Unknown error', + timestamp: new Date().toISOString(), + }, + }); + }Also applies to: 185-194
__tests__/getOrderBookDepth.test.ts (7)
142-159: Tests assume Promise, but handler now emits via callback and returns voidUpdate this test to capture the emitted Content via the callback instead of relying on a return value.
Apply:
- const result = (await getOrderBookDepthAction.handler( - mockRuntime, - mockMessage, - mockState - )) as Content; + let result!: Content; + await getOrderBookDepthAction.handler( + mockRuntime, + mockMessage, + mockState, + {}, + (c) => { + result = c; + } + );
189-209: Same issue: switch to callback capture; also assert on callback contentThis path should mirror the callback pattern and then assert on result.text.
Apply:
- const result = (await getOrderBookDepthAction.handler( - mockRuntime, - mockMessage, - mockState - )) as Content; + let result!: Content; + await getOrderBookDepthAction.handler( + mockRuntime, + mockMessage, + mockState, + {}, + (c) => { + result = c; + } + );
270-276: Should not throw when CLOB_API_URL missing; assert error via callback insteadHandler emits an error via callback and returns. Replace rejects.toThrow with callback assertions.
- await expect( - getOrderBookDepthAction.handler(mockRuntime, mockMessage, mockState) - ).rejects.toThrow('CLOB_API_URL is required in configuration.'); + const cb = vi.fn(); + await getOrderBookDepthAction.handler(mockRuntime, mockMessage, mockState, {}, cb); + expect(cb).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining('CLOB_API_URL is required in configuration.'), + actions: ['GET_ORDER_BOOK_DEPTH'], + }) + );
278-290: LLM error path: assert callback error content, not thrown exceptionUpdate to callback assertions.
- await expect( - getOrderBookDepthAction.handler(mockRuntime, mockMessage, mockState) - ).rejects.toThrow( - 'Token identifiers not found. Please specify one or more token IDs for order book depth.' - ); + const cb = vi.fn(); + await getOrderBookDepthAction.handler(mockRuntime, mockMessage, mockState, {}, cb); + expect(cb).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining('❌ **Error**'), + actions: ['GET_ORDER_BOOK_DEPTH'], + }) + );
354-365: No order books found: assert callback error contentConvert from thrown error to callback assertion.
- await expect( - getOrderBookDepthAction.handler(mockRuntime, mockMessage, mockState) - ).rejects.toThrow('No order books found for the provided token IDs: 123456'); + const cb = vi.fn(); + await getOrderBookDepthAction.handler(mockRuntime, mockMessage, mockState, {}, cb); + expect(cb).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining('No order books found for the provided token IDs: 123456'), + actions: ['GET_ORDER_BOOK_DEPTH'], + }) + );
380-399: Callback success test asserts wrong argument (undefined); assert on first callback argSince handler returns void,
resultis undefined. Assert against the callback payload.- const result = await getOrderBookDepthAction.handler( + await getOrderBookDepthAction.handler( mockRuntime, mockMessage, mockState, {}, mockCallback ); - - expect(mockCallback).toHaveBeenCalledWith(result); - expect(mockCallback).toHaveBeenCalledTimes(1); + expect(mockCallback).toHaveBeenCalledTimes(1); + const payload = mockCallback.mock.calls[0][0] as Content; + expect(payload.text).toContain('📊 **Order Book Depth Summary**'); + expect(payload.actions).toContain('GET_ORDER_BOOK_DEPTH');
401-420: Callback error test: remove rejects.toThrow and assert on callback payloadAlign with callback-driven errors.
- await expect( - getOrderBookDepthAction.handler(mockRuntime, mockMessage, mockState, {}, mockCallback) - ).rejects.toThrow(); - - expect(mockCallback).toHaveBeenCalledWith( + await getOrderBookDepthAction.handler( + mockRuntime, + mockMessage, + mockState, + {}, + mockCallback + ); + expect(mockCallback).toHaveBeenCalledWith( expect.objectContaining({ text: expect.stringContaining('❌ **Error**'), - actions: ['GET_ORDER_BOOK_DEPTH'], + actions: ['GET_ORDER_BOOK_DEPTH'], data: expect.objectContaining({ error: expect.any(String) }), }) );src/actions/getOrderBookDepth.ts (3)
237-241: Unify emitted action identifier with Action.nameSuccess payload uses 'POLYMARKET_GET_ORDER_BOOK_DEPTH' while Action.name is 'GET_ORDER_BOOK_DEPTH'. Use a single identifier to avoid brittle tests and client logic.
- actions: ['POLYMARKET_GET_ORDER_BOOK_DEPTH'], + actions: ['GET_ORDER_BOOK_DEPTH'],
274-277: Unify error payload action identifierSame inconsistency in the error path; align to 'GET_ORDER_BOOK_DEPTH'.
- actions: ['POLYMARKET_GET_ORDER_BOOK_DEPTH'], + actions: ['GET_ORDER_BOOK_DEPTH'],
46-53: Breaking API change — update callers/tests for Promise + callback and fix action-name mismatch
- Tests call getOrderBookDepthAction.handler(...) without a callback and assert rejects.toThrow — update those tests to pass a callback (or restore previous throw/return behavior). See tests/getOrderBookDepth.test.ts lines: 274, 286, 306, 364, 377, 410-412.
- Action identifier is inconsistent: src/actions/getOrderBookDepth.ts defines name 'GET_ORDER_BOOK_DEPTH' (line 20) but several Content objects use 'POLYMARKET_GET_ORDER_BOOK_DEPTH' (lines ~239, 276, 303, 318, 333). Align to a single identifier (preferably 'GET_ORDER_BOOK_DEPTH') and update consumers/tests accordingly.
🧹 Nitpick comments (36)
.gitignore (1)
3-4: Ignore all env variants and keep an example checked inAdd patterns for .env.* and keep an example unignored.
-.env +.env +.env.* +!.env.example + +# Turbo local cache .turbosrc/utils/llmHelpers.ts (2)
28-33: Clear the timeout after race to avoid a dangling timerThe timer isn’t cleared when the LLM resolves first; repeated calls can leak timers.
- const timeoutPromise = new Promise<never>((_, reject) => { - setTimeout( - () => reject(new Error(`LLM call timed out after ${timeoutMs / 1000} seconds`)), - timeoutMs - ); - }); + let tid: ReturnType<typeof setTimeout> | undefined; + const timeoutPromise = new Promise<never>((_, reject) => { + tid = setTimeout( + () => reject(new Error(`LLM call timed out after ${timeoutMs / 1000} seconds`)), + timeoutMs + ); + }); const result = (await Promise.race([llmPromise, timeoutPromise])) as T; logger.info(`[${actionName}] LLM parameter extraction completed`); - logger.debug(`[${actionName}] Parsed LLM parameters: ${JSON.stringify(result)}`); + if (tid) clearTimeout(tid); + logger.debug(`[${actionName}] Parsed LLM parameters:`, result);Also applies to: 42-47
45-45: Prefer structured logging over pre-stringifyingPassing the object lets the logger handle redaction/serialization and avoids huge single-line logs.
-logger.debug(`[${actionName}] Parsed LLM parameters: ${JSON.stringify(result)}`); +logger.debug(`[${actionName}] Parsed LLM parameters:`, result);src/actions/revokeApiKey.ts (5)
203-214: Add a network timeout to the DELETE requestPrevent hanging requests with
AbortController; consistent with your LLM timeout approach.- const deleteResponse = await fetch(`${clobApiUrl}${requestPath}`, { - method: 'DELETE', - headers: { - 'Content-Type': 'application/json', - POLY_ADDRESS: address, - POLY_SIGNATURE: signature, - POLY_TIMESTAMP: timestamp, - POLY_API_KEY: apiKey, - POLY_PASSPHRASE: apiPassphrase, - }, - body: JSON.stringify({ key: apiKeyId }), - }); + const ac = new AbortController(); + const abortId = setTimeout(() => ac.abort('timeout'), 15_000); + try { + const deleteResponse = await fetch(`${clobApiUrl}${requestPath}`, { + method: 'DELETE', + headers: { + 'Content-Type': 'application/json', + POLY_ADDRESS: address, + POLY_SIGNATURE: signature, + POLY_TIMESTAMP: timestamp, + POLY_API_KEY: apiKey, + POLY_PASSPHRASE: apiPassphrase, + }, + body: JSON.stringify({ key: apiKeyId }), + signal: ac.signal, + }); + // ... keep existing handling + } finally { + clearTimeout(abortId); + }
130-156: Validate API key ID format (UUID) before calling the APIAvoids unnecessary requests and clearer feedback.
const apiKeyId = apiKeyIdString.trim(); - logger.info(`[revokeApiKeyAction] Extracted API key ID: ${apiKeyId}`); + const uuidRe = + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; + if (!uuidRe.test(apiKeyId)) { + if (callback) { + callback({ + text: + '❌ **API Key Revocation Failed**\n\n**Error**: Invalid API key ID format. Expect UUID.', + action: 'POLYMARKET_DELETE_API_KEY', + data: { success: false, error: 'Invalid API key format' }, + }); + } + return; + } + const maskedId = apiKeyId.replace(/.(?=.{4}$)/g, '•'); + logger.info(`[revokeApiKeyAction] Extracted API key ID: ${maskedId}`);
65-80: Strengthen validate(): check presence of CLOB creds tooEarly-return if API key/secret/passphrase are missing.
validate: async (runtime: IAgentRuntime, message: Memory): Promise<boolean> => { @@ - if (!privateKey) { + if (!privateKey) { logger.error('[revokeApiKeyAction] No private key found in environment'); return false; } + const apiKey = runtime.getSetting('CLOB_API_KEY'); + const apiSecret = runtime.getSetting('CLOB_API_SECRET'); + const apiPassphrase = runtime.getSetting('CLOB_API_PASSPHRASE'); + if (!apiKey || !apiSecret || !apiPassphrase) { + logger.error('[revokeApiKeyAction] Missing CLOB API credentials'); + return false; + }
82-90: Align handler signature with optional params
stateandcallbackare treated as optional below; type them as such for consistency with other actions.- handler: async ( - runtime: IAgentRuntime, - message: Memory, - state: State, - options: any, - callback: HandlerCallback - ): Promise<void> => { + handler: async ( + runtime: IAgentRuntime, + message: Memory, + state?: State, + options?: Record<string, unknown>, + callback?: HandlerCallback + ): Promise<void> => {
2-2: Remove unused import
initializeClobClientisn’t used here.-import { initializeClobClient } from '../utils/clobClient';src/types.ts (1)
426-429: TightenOpenOrdertypes (optional)If possible, restrict
type/statusto known literals or reuseOrderStatus.- type: string; // e.g., "LIMIT" - status: string; // e.g., "OPEN" - could create an enum if more statuses are known for open orders + type: 'LIMIT' | 'MARKET'; + status: OrderStatus; // if API aligns; otherwise a narrowed union like: 'OPEN' | 'PARTIALLY_FILLED'src/actions/getSpread.ts (2)
129-133: Guard against NaN spread valuesIf the API returns a non-numeric string, formatting will produce
NaN.- const spreadValue = parseFloat(spreadResponse.spread); + const spreadValue = parseFloat(spreadResponse.spread); + if (Number.isNaN(spreadValue)) { + const errorContent: Content = { + text: '❌ **Error**: Invalid spread value returned by API.', + actions: ['POLYMARKET_GET_SPREAD'], + data: { tokenId, raw: spreadResponse.spread }, + }; + if (callback) await callback(errorContent); + return; + }
63-85: Prefer explicit param over LLM when providedIf
options?.tokenIdexists, use it and skip LLM/regex.- let tokenId: string; + let tokenId: string; + const opt = (options || {}) as { tokenId?: string }; + if (opt.tokenId) { + tokenId = opt.tokenId; + } else { try { // Use LLM to extract parameters const llmResult = await callLLMWithTimeout<{ tokenId?: string; error?: string }>( @@ - if (!tokenId) { + if (!tokenId) { throw new Error('Token ID not found'); } - } catch (error) { + } catch (error) { logger.warn('[getSpreadAction] LLM extraction failed, trying regex fallback'); + // ... + }src/actions/getSimplifiedMarkets.ts (2)
141-151: Guard against non-string token_id before slicetoken_id is validated for presence but not type. Coerce to string to avoid runtime errors if upstream shape changes.
- responseText += ` • Tokens: ${yesToken.outcome || 'Token1'} (${yesToken.token_id.slice(0, 8)}...) / ${noToken.outcome || 'Token2'} (${noToken.token_id.slice(0, 8)}...)\n`; + const yesId = String(yesToken.token_id); + const noId = String(noToken.token_id); + responseText += ` • Tokens: ${yesToken.outcome || 'Token1'} (${yesId.slice(0, 8)}...) / ${noToken.outcome || 'Token2'} (${noId.slice(0, 8)}...)\n`;
162-165: Avoid magic cursor sentinelConsider a shared constant (e.g., END_CURSOR = 'LTE=') to prevent typos and ease future changes.
src/actions/getOpenMarkets.ts (1)
117-127: Variable name/log label mismatchactiveOnly actually counts active && closed. Rename to activeButClosed (and update log) for clarity.
- const activeOnly = allMarkets.filter((m) => m.active === true && m.closed === true).length; + const activeButClosed = allMarkets.filter((m) => m.active === true && m.closed === true).length; ... - logger.info( - `[getOpenMarkets] Overlap analysis: activeAndNonClosed=${activeAndNonClosed}, activeButClosed=${activeOnly}, nonClosedButInactive=${nonClosedOnly}` - ); + logger.info( + `[getOpenMarkets] Overlap analysis: activeAndNonClosed=${activeAndNonClosed}, activeButClosed=${activeButClosed}, nonClosedButInactive=${nonClosedOnly}` + );src/actions/getOrderBookSummary.ts (2)
15-18: Use the Side enum instead of string literals with any-castsRemoves unsafe casts and uses the enum you defined.
- const [buyBook, sellBook] = await Promise.all([ - clobClient.getOrderBooks([{ token_id: tokenId, side: 'buy' as any }]), - clobClient.getOrderBooks([{ token_id: tokenId, side: 'sell' as any }]), - ]); + const [buyBook, sellBook] = await Promise.all([ + clobClient.getOrderBooks([{ token_id: tokenId, side: Side.BUY }]), + clobClient.getOrderBooks([{ token_id: tokenId, side: Side.SELL }]), + ]);Also applies to: 171-173
123-129: Unreachable catch branchThe specific “Token identifier not found…” error is never thrown (you return earlier). Drop this check to simplify.
- if ( - error instanceof Error && - error.message === - 'Token identifier not found. Please specify a token ID for the order book.' - ) { - return; - }src/utils/clobClient.ts (1)
51-56: Optional: make chain ID configurableHardcoding 137 is fine for Polymarket, but a configurable CHAIN_ID eases future migrations.
- logger.info(`[initializeClobClient] Chain ID: 137`); - const client = new ClobClient( - clobApiUrl, - 137, // Polygon chain ID + const chainId = Number(runtime.getSetting('CHAIN_ID') || 137); + logger.info(`[initializeClobClient] Chain ID: ${chainId}`); + const client = new ClobClient( + clobApiUrl, + chainId, // Polygon chain ID enhancedWallet as any, undefined, // No API creds for this basic client clobWsUrl // Pass WebSocket URL );- logger.info(`[initializeClobClientWithCreds] Chain ID: 137`); - const client = new ClobClient( - clobApiUrl, - 137, // Polygon chain ID + const chainId = Number(runtime.getSetting('CHAIN_ID') || 137); + logger.info(`[initializeClobClientWithCreds] Chain ID: ${chainId}`); + const client = new ClobClient( + clobApiUrl, + chainId, // Polygon chain ID enhancedWallet as any, creds, // API credentials for L2 authentication clobWsUrl // Pass WebSocket URL );Also applies to: 137-143
src/actions/retrieveAllMarkets.ts (1)
117-131: Locale-stable datesUse an explicit locale for deterministic formatting across environments.
- new Date(market.end_date_iso).toLocaleDateString() + new Date(market.end_date_iso).toLocaleDateString('en-US')src/actions/placeOrder.ts (3)
256-264: postOrder error should also go to callbackMirror the above for postOrder so failures are user-visible.
- } catch (postError) { - logger.error(`[placeOrderAction] Error posting order:`, postError); - logger.error( - `Failed to submit order: ${postError instanceof Error ? postError.message : 'Unknown error'}` - ); - return; - } + } catch (postError) { + const msg = postError instanceof Error ? postError.message : 'Unknown error'; + logger.error(`[placeOrderAction] Error posting order:`, postError); + const errorContent: Content = { + text: `❌ Failed to submit order: ${msg}`, + actions: ['POLYMARKET_PLACE_ORDER'], + data: { error: msg, tokenId, orderType }, + }; + if (callback) await callback(errorContent); + return; + }
230-231: Typing: avoid broad any on orderArgsIf ClobClient.createOrder expects a specific shape, remove the any cast to catch mistakes at compile time.
- signedOrder = await (client as ClobClient).createOrder(orderArgs as any); + type CreateOrderArgs = Parameters<ClobClient['createOrder']>[0]; + signedOrder = await (client as ClobClient).createOrder(orderArgs as CreateOrderArgs);
256-257: Prefer typed client over anyCasting to any loses safety on a critical external call.
- orderResponse = await (client as any).postOrder(signedOrder, orderType as OrderType); + orderResponse = await (client as ClobClient).postOrder(signedOrder, orderType as OrderType);src/actions/getPriceHistory.ts (2)
135-139: Avoid any on clob client; add a narrow interface insteadKeep type safety on the API call.
-const priceHistory = await (clobClient as any).getPricesHistory({ +type PriceHistoryClient = { + getPricesHistory(params: { token_id: string; interval: PriceHistoryInterval | string }): Promise<PricePoint[]>; +}; +const priceHistory = await (clobClient as PriceHistoryClient).getPricesHistory({ token_id: params.tokenId, interval: interval as any, });
149-161: Callback payload shape inconsistent with other actionsOther actions send { text, actions, data }. Here it’s { text, content: { action, ... } }. Standardize to reduce downstream parsing branches.
- if (callback) { - await callback({ - text: responseMessage, - content: { - action: 'POLYMARKET_PRICE_HISTORY_RETRIEVED', - tokenId: params.tokenId, - interval: interval, - priceHistory: priceHistory, - pointsCount: priceHistory?.length || 0, - timestamp: new Date().toISOString(), - }, - }); - } + if (callback) { + await callback({ + text: responseMessage, + actions: ['POLYMARKET_PRICE_HISTORY_RETRIEVED'], + data: { + tokenId: params.tokenId, + interval, + priceHistory, + pointsCount: priceHistory?.length || 0, + timestamp: new Date().toISOString(), + }, + }); + }src/actions/handleAuthentication.ts (1)
185-188: Persisted secrets: mirror parameters in plugin config and ensure maskingYou set CLOB_API_KEY/SECRET/PASSPHRASE at runtime; add these as sensitive pluginParameters for visibility in agent UIs and to ensure masking.
See package.json comment for the exact fields to add.
package.json (2)
68-100: Expose stored secrets in pluginParameters (sensitive) for parity with runtime settingsAdd CLOB_API_SECRET and CLOB_API_PASSPHRASE to match handleAuthentication persistence.
"CLOB_API_KEY": { "type": "string", "description": "API key for authenticating requests to the Polymarket CLOB API.", "required": false, "sensitive": true }, + "CLOB_API_SECRET": { + "type": "string", + "description": "API secret for the Polymarket CLOB API.", + "required": false, + "sensitive": true + }, + "CLOB_API_PASSPHRASE": { + "type": "string", + "description": "API passphrase for the Polymarket CLOB API.", + "required": false, + "sensitive": true + }, "POLYMARKET_PRIVATE_KEY": {
103-103: Align npmPackage with namenpmPackage is still unscoped; align with the new scoped name for consistency in registries.
- "npmPackage": "plugin-polymarket" + "npmPackage": "@elizaos/plugin-polymarket"__tests__/getOrderBookDepth.test.ts (7)
211-239: Empty book case: convert to callback; keep assertionsSame callback change as above.
Apply:
- const result = (await getOrderBookDepthAction.handler( - mockRuntime, - mockMessage, - mockState - )) as Content; + let result!: Content; + await getOrderBookDepthAction.handler( + mockRuntime, + mockMessage, + mockState, + {}, + (c) => { + result = c; + } + );
241-268: Mixed books case: convert to callback; keep assertionsSame callback change as above.
- const result = (await getOrderBookDepthAction.handler( - mockRuntime, - mockMessage, - mockState - )) as Content; + let result!: Content; + await getOrderBookDepthAction.handler( + mockRuntime, + mockMessage, + mockState, + {}, + (c) => { + result = c; + } + );
292-310: No valid token IDs: assert callback error contentConvert to callback + content assertions.
- await expect( - getOrderBookDepthAction.handler(mockRuntime, mockMessageWithoutToken, mockState) - ).rejects.toThrow( - 'Unable to extract token IDs from your message. Please provide valid token IDs.' - ); + const cb = vi.fn(); + await getOrderBookDepthAction.handler( + mockRuntime, + mockMessageWithoutToken, + mockState, + {}, + cb + ); + expect(cb).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining( + 'Unable to extract token IDs from your message. Please provide valid token IDs.' + ), + actions: ['GET_ORDER_BOOK_DEPTH'], + }) + );
312-332: Regex fallback case: convert to callback captureUpdate to callback capture and keep assertions.
- const result = (await getOrderBookDepthAction.handler( - mockRuntime, - mockMessageWithTokens, - mockState - )) as Content; + let result!: Content; + await getOrderBookDepthAction.handler( + mockRuntime, + mockMessageWithTokens, + mockState, + {}, + (c) => { + result = c; + } + );
334-352: Invalid token filter: you don’t need the return; optionally capture via callback for consistencyOptional: switch to callback pattern; existing getOrderBooks call assertion is fine either way.
- const result = (await getOrderBookDepthAction.handler( - mockRuntime, - mockMessage, - mockState - )) as Content; + await getOrderBookDepthAction.handler( + mockRuntime, + mockMessage, + mockState, + {}, + () => {} + );
367-378: CLOB client failure: assert callback error contentConvert to callback assertion.
- await expect( - getOrderBookDepthAction.handler(mockRuntime, mockMessage, mockState) - ).rejects.toThrow('API Error'); + const cb = vi.fn(); + await getOrderBookDepthAction.handler(mockRuntime, mockMessage, mockState, {}, cb); + expect(cb).toHaveBeenCalledWith( + expect.objectContaining({ + text: expect.stringContaining('API Error'), + actions: ['GET_ORDER_BOOK_DEPTH'], + }) + );
422-443: Summary stats test: convert to callback captureUpdate to callback and keep assertions.
- const result = (await getOrderBookDepthAction.handler( - mockRuntime, - mockMessage, - mockState - )) as Content; - - const data = result.data as any; + let payload!: Content; + await getOrderBookDepthAction.handler( + mockRuntime, + mockMessage, + mockState, + {}, + (c) => { + payload = c; + } + ); + const data = payload.data as any;src/actions/getOrderBookDepth.ts (3)
183-188: Avoidanyfor request params; add a minimal local typePreserve type-safety without re‑introducing the removed BookParams type.
- // Prepare book parameters - cast to any since the library accepts it without side - const bookParams = tokenIds.map((tokenId) => ({ token_id: tokenId })); + // Prepare book parameters + type BookParam = { token_id: string }; + const bookParams: BookParam[] = tokenIds.map((tokenId) => ({ token_id: tokenId })); @@ - const orderBooks: OrderBook[] = await clobClient.getOrderBooks(bookParams as any); + const orderBooks: OrderBook[] = await clobClient.getOrderBooks(bookParams as any);
125-128: Dead branch / misleading comment in catchThis branch checks for an error message that isn’t thrown in the try block; the LLM “not found” case already returns earlier. Remove or clarify.
- // Check if this is our specific error message and re-throw it - if (error instanceof Error && error.message.includes('Token identifiers not found')) { - return; - } + // LLM error path already handled above.
300-338: Examples: align action identifier to the one emittedUpdate examples to prevent confusion for integrators and tests.
- actions: ['POLYMARKET_GET_ORDER_BOOK_DEPTH'], + actions: ['GET_ORDER_BOOK_DEPTH'], @@ - actions: ['POLYMARKET_GET_ORDER_BOOK_DEPTH'], + actions: ['GET_ORDER_BOOK_DEPTH'], @@ - actions: ['POLYMARKET_GET_ORDER_BOOK_DEPTH'], + actions: ['GET_ORDER_BOOK_DEPTH'],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (19)
.gitignore(1 hunks)__tests__/getOrderBookDepth.test.ts(2 hunks)package.json(3 hunks)packages/registry/index.json(0 hunks)packages/registry/packages/@elizaos/plugin-polymarket/0.1.0.json(0 hunks)src/actions/getOpenMarkets.ts(5 hunks)src/actions/getOrderBookDepth.ts(8 hunks)src/actions/getOrderBookSummary.ts(7 hunks)src/actions/getPriceHistory.ts(6 hunks)src/actions/getSamplingMarkets.ts(5 hunks)src/actions/getSimplifiedMarkets.ts(4 hunks)src/actions/getSpread.ts(4 hunks)src/actions/handleAuthentication.ts(6 hunks)src/actions/placeOrder.ts(6 hunks)src/actions/retrieveAllMarkets.ts(5 hunks)src/actions/revokeApiKey.ts(2 hunks)src/types.ts(3 hunks)src/utils/clobClient.ts(8 hunks)src/utils/llmHelpers.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/registry/index.json
- packages/registry/packages/@elizaos/plugin-polymarket/0.1.0.json
🔇 Additional comments (18)
src/types.ts (1)
441-447: LGTM – params look consistent
to_timestampas seconds matches surrounding types.src/actions/getSpread.ts (1)
56-56: API surface change to Promise + callback: goodThe callback-first pattern eliminates thrown errors from bubbling; consistent with the PR’s direction.
src/actions/getOpenMarkets.ts (1)
11-14: Import style consistency (.js extension)Same note as in getSamplingMarkets.ts—verify module resolution settings or standardize imports.
src/actions/getOrderBookSummary.ts (1)
179-182: Verify bid/ask mapping directionYou map bids from sellBook and asks from buyBook. Confirm this matches Polymarket’s API semantics, or flip if needed.
src/utils/clobClient.ts (1)
100-105: Logs: safe, but double‑check policyOnly previews the API key—good. Confirm org policy allows logging wallet address and URLs at info level in production; consider downgrading to debug in prod.
src/actions/getSamplingMarkets.ts (2)
11-14: Confirm tsconfig supports ESM .js import specifiers or standardize importsTypeScript files are importing with .js extensions (found):
- src/actions/getSamplingMarkets.ts:11–13
- src/actions/getPriceHistory.ts:11–12
- src/actions/getOpenMarkets.ts:11–13
- src/actions/getClobMarkets.ts:12–14
- e2e/starter-plugin.test.ts:1
Either enable Node ESM resolution in tsconfig (NodeNext/Bundler-style settings) or remove the .js extensions to avoid build/runtime breaks.
58-59: Promise change — verified safeSearched the repo (excluding tests) for .handler calls and any await …handler comparisons — no callers check for true/false; @elizaos/core's Handler returns a Promise/ActionResult, so returning Promise is acceptable. (eliza.how)
src/actions/retrieveAllMarkets.ts (2)
53-66: Good early-return error path with callbackConfiguration validation now reports via callback and exits cleanly. Looks good.
48-49: Handler now returns Promise — verify callers/tests don't rely on a returned ContentRepo mixes handler styles (some handlers return Promise, many use callback + Promise). retrieveAllMarketsAction uses callback + Promise (handler signature at src/actions/retrieveAllMarkets.ts lines ~48–49). Confirm no direct callers or tests await/consume a returned Content — check src/tests/retrieveAllMarkets.test.ts (handler call sites around lines ~105,142,171,188) and plugin registration (src/plugin.ts:205). If any callers expect Content, either update those callers or return Content from this handler.
src/actions/placeOrder.ts (1)
76-77: Callback-only flow aligns with repo’s patternSwitch to Promise and consistent early returns looks good.
Also applies to: 93-94, 195-196, 361-362, 394-394
src/actions/getPriceHistory.ts (2)
9-9: Importing ActionExample as a type — good cleanupThis avoids runtime imports. LGTM.
230-231: satisfies for examples — nice type safety winGood modernization; keeps examples structurally sound.
src/actions/handleAuthentication.ts (3)
81-91: Clear, structured ActionResult flowsConsistent success/error shaping with callback invocation. Looks good.
Also applies to: 156-163, 207-215, 219-225
67-68: Confirm framework supports non-void action handlers (Promise)Handler now returns Promise; repo search did not find the Action/handler type declaration (it's imported from @elizaos/core). Confirm that Action/HandlerCallback permits a non-void/Promise return and that the runtime/callers await or use that value — if not, change the handler to return void or update the framework types/callers.
Location: src/actions/handleAuthentication.ts:67-68
121-141: createOrDeriveApiKey: add runtime fallback to deriveApiKey and normalize responseREADME examples reference createOrDeriveApiKey; deriveApiKey (GET /auth/derive-api-key) returns key/secret/passphrase — add a runtime fallback and normalize the response shape.
- const rawResponse = await clientSignerInstance.createOrDeriveApiKey(); + const anyClient = clientSignerInstance as any; + const rawResponse = + typeof anyClient.createOrDeriveApiKey === 'function' + ? await anyClient.createOrDeriveApiKey() + : await anyClient.deriveApiKey(); ... - derivedApiCredsFromClient = rawResponse as unknown as RawDerivedApiKeyResponse; + const normalized = { + key: (rawResponse.key ?? rawResponse.apiKey) as string, + secret: (rawResponse.secret ?? rawResponse.apiSecret) as string, + passphrase: rawResponse.passphrase as string, + label: rawResponse.label as string | undefined, + expiration_timestamp_ms: rawResponse.expiration_timestamp_ms as number | undefined, + }; + derivedApiCredsFromClient = normalized as RawDerivedApiKeyResponse;package.json (2)
2-5: Package rename and version bump — LGTMScoped name and 1.0.0 version align with the API changes.
32-35: Pinning explicit @elizaOS ranges — verify compatibilityFile: package.json Lines: 32-35
"@elizaos/cli": "^1.5.9-alpha.1", "@elizaos/core": "^1.5.9-alpha.1", "@elizaos/plugin-sql": "^1.5.9-alpha.1",These three deps use a careted alpha range. Confirm downstream agents run the same alpha; if you need reproducible installs, pin exact versions (1.5.9-alpha.1) or rely on a committed lockfile.
__tests__/getOrderBookDepth.test.ts (1)
175-187: Align expectations and params; keep, but convert call-site to callback patternKeep the expectedParams assertion, but ensure the test above captures result via callback and that assertions on result come from the callback, not the return.
No change needed here beyond calling the handler with a callback as shown in the prior diff.
src/actions/getOrderBookSummary.ts
Outdated
| ): Promise<void> => { | ||
| logger.info('[getOrderBookSummaryAction] Handler called!'); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Contract changed to Promise: verify call sites
Ensure no callers expect a returned Content.
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '(const\s+\w+\s*=\s*await\s+getOrderBookSummaryAction\.handler\()|return\s+await\s+getOrderBookSummaryAction\.handler\(' srcLength of output: 0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for occurrences of 'getOrderBookSummaryAction'..."
rg -n --hidden -S -C3 '\bgetOrderBookSummaryAction\b' || true
echo
echo "Attempting to show src/actions/getOrderBookSummary.ts (if present)..."
if [ -f src/actions/getOrderBookSummary.ts ]; then
sed -n '1,240p' src/actions/getOrderBookSummary.ts || true
else
echo "File not found: src/actions/getOrderBookSummary.ts"
fi
echo
echo "Searching for any '.handler(' usages (broader scan)..."
rg -n --hidden -S -C3 '\.handler\(' || true
echo
echo "Searching for plain 'getOrderBookSummary' occurrences..."
rg -n --hidden -S -C3 '\bgetOrderBookSummary\b' || trueLength of output: 124401
Handler now returns Promise — callers still expect a returned Content
- Problem: src/actions/getOrderBookSummary.ts handler signature was changed to Promise (src/actions/getOrderBookSummary.ts:57); callers/docs/tests still await and use the returned result (e.g. README.md:460–463; tests/getOrderBookSummary.test.ts:119, 332, 417).
- Action required: either restore the handler return (return Content/result) or update all callers (README, tests, and any code awaiting a result) to use the callback pattern and adjust assertions.
🤖 Prompt for AI Agents
In src/actions/getOrderBookSummary.ts around lines 57 to 58, the handler was
changed to return Promise<void> but callers/tests/docs still expect a returned
Content; restore the original behavior by changing the handler signature back to
Promise<Content> (or the correct Content type used elsewhere) and ensure the
function returns the computed content/result at the end (add a final return of
the response object instead of void). If you deliberately want to keep
Promise<void>, update all callers/tests/docs to stop awaiting a return and adapt
assertions to use the callback pattern; otherwise implement the signature and
return fix so existing callers/tests continue to work.
src/actions/getPriceHistory.ts
Outdated
| ): Promise<void> => { | ||
| try { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Signature change to Promise — verify no callers depend on boolean
Previously returned boolean; ensure nothing consumes a boolean result.
🏁 Script executed:
#!/bin/bash
# Find callsites or tests expecting a boolean from getPriceHistory
rg -nP -C2 '\bgetPriceHistory\b' --type=ts | sed -n '1,200p'Length of output: 9790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/actions/getPriceHistory.ts (first 240 lines) ==="
sed -n '1,240p' src/actions/getPriceHistory.ts || echo "file not found: src/actions/getPriceHistory.ts"
echo
echo "=== __tests__/getPriceHistory.test.ts (first 420 lines) ==="
sed -n '1,420p' __tests__/getPriceHistory.test.ts || echo "file not found: __tests__/getPriceHistory.test.ts"
echo
echo "=== occurrences of getPriceHistory.handler ==="
rg -n "getPriceHistory\.handler" -S || true
echo
echo "=== occurrences of 'const result = await getPriceHistory.handler' ==="
rg -n "const result = await getPriceHistory\.handler" -S || true
echo
echo "=== any expectations using 'result' in that test file ==="
rg -n "expect\(\s*result" __tests__/getPriceHistory.test.ts || trueLength of output: 20950
Handler now returns void but callers/tests expect a boolean — fix required
- src/actions/getPriceHistory.ts — handler signature is currently
): Promise<void> =>(returns nothing). Tests and callers await a boolean result; change toPromise<boolean>and return true on success / false on failure, or update all callers/tests to not rely on a boolean. - tests/getPriceHistory.test.ts — multiple tests assert
expect(result).toBe(true|false)and validate specific callback payloads and clob API call shape (lines ~89–317). Align tests with the implementation or adapt the implementation to match tests (callback action strings, getPricesHistory args). - README.md (line ~349) also awaits the handler result; update example to match chosen contract.
🤖 Prompt for AI Agents
In src/actions/getPriceHistory.ts around lines 95-96, the handler currently
returns Promise<void> but tests and callers expect a boolean; change the
function signature to Promise<boolean> and ensure the implementation returns
true on successful completion and false on any handled failure (catch block).
Also audit and update related call sites and README examples (e.g.,
__tests__/getPriceHistory.test.ts and README.md line ~349) to rely on the
boolean return contract and keep existing callback payloads and clob API call
shapes unchanged so tests asserting expect(result).toBe(true|false) and payload
shapes continue to pass.
src/actions/getSimplifiedMarkets.ts
Outdated
| ): Promise<void> => { | ||
| logger.info('[getSimplifiedMarketsAction] Handler called!'); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Contract changed to Promise: verify call sites don’t rely on returned Content
Good shift to callback-driven flow. Please confirm no callers still await a returned Content or boolean.
🏁 Script executed:
#!/bin/bash
# Find places capturing/returning handler results (should be unused now)
rg -nP -C2 '(const\s+\w+\s*=\s*await\s+\w+\.handler\()|return\s+await\s+\w+\.handler\(' srcLength of output: 1980
Contract changed to Promise: callers still await handler — update callers/tests or revert
Several tests still capture and await the handler return; update those callers to not rely on a returned value or restore the previous return type.
- src/tests/getSimplifiedMarkets.test.ts:145 — const result = await getSimplifiedMarketsAction.handler(...)
- src/tests/getSimplifiedMarkets.test.ts:183 — const result = await getSimplifiedMarketsAction.handler(...)
- src/tests/getSimplifiedMarkets.test.ts:295 — const result = await getSimplifiedMarketsAction.handler(...)
- src/tests/retrieveAllMarkets.test.ts:105 — const result = await retrieveAllMarketsAction.handler(...)
- src/tests/retrieveAllMarkets.test.ts:142 — const result = await retrieveAllMarketsAction.handler(...)
🤖 Prompt for AI Agents
In src/actions/getSimplifiedMarkets.ts around lines 54-55, the handler signature
was changed to return Promise<void> while multiple callers/tests still await and
use the handler's return value; either restore the previous non-void return type
or update the callers/tests to stop awaiting/using a result. To fix: choose one
approach—(A) revert the handler's return type to the previous Promise<...> and
return the expected value so existing tests keep working, ensuring the exported
type/signature matches callers; or (B) keep Promise<void> and update the listed
tests (src/__tests__/getSimplifiedMarkets.test.ts lines ~145,183,295 and
src/__tests__/retrieveAllMarkets.test.ts lines ~105,142) to call the handler
without awaiting a return value and adjust assertions to inspect side effects or
mocked responses instead of a returned result.
| const response = await (clobClient as any).getMarkets( | ||
| filterParams?.next_cursor || '', | ||
| filterParams | ||
| ); |
There was a problem hiding this comment.
Type mismatch: filterParams.next_cursor isn’t in MarketFilters
You’re reading filterParams.next_cursor while filterParams: MarketFilters = {} doesn’t declare it. This will either break type-checking or hide bugs. Define a local nextCursor or extend the type.
Apply one of these diffs (Option A recommended):
+type MarketFiltersWithCursor = MarketFilters & { next_cursor?: string };
-let filterParams: MarketFilters = {};
+let filterParams: MarketFiltersWithCursor = {};
...
-const response = await (clobClient as any).getMarkets(
- filterParams?.next_cursor || '',
- filterParams
-);
+const response = await (clobClient as any).getMarkets(
+ filterParams.next_cursor ?? '',
+ filterParams
+);Or minimally:
-const response = await (clobClient as any).getMarkets(
- filterParams?.next_cursor || '',
- filterParams
-);
+const nextCursor = (filterParams as any).next_cursor ?? '';
+const response = await (clobClient as any).getMarkets(nextCursor, filterParams);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const response = await (clobClient as any).getMarkets( | |
| filterParams?.next_cursor || '', | |
| filterParams | |
| ); | |
| const nextCursor = (filterParams as any).next_cursor ?? ''; | |
| const response = await (clobClient as any).getMarkets(nextCursor, filterParams); |
🤖 Prompt for AI Agents
In src/actions/retrieveAllMarkets.ts around lines 104 to 107, the code reads
filterParams.next_cursor even though filterParams is typed as MarketFilters
which does not declare next_cursor; create a local const nextCursor =
(filterParams as any)?.next_cursor ?? '' and pass nextCursor to getMarkets
(Option A), or alternatively extend MarketFilters to include next_cursor and
update its type usage so the property is properly declared and type-checked;
ensure you do not mutate filterParams and that the value defaults to '' when
undefined.
| market?: string; // Market condition ID | ||
| assetId?: string; // Asset ID (token ID) | ||
| address?: string; // User address | ||
| nextCursor?: string; // Pagination cursor | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Camel-cased params require mapping at the client boundary
Ensure getOpenOrders translates assetId → asset_id and nextCursor → next_cursor when calling the API.
🏁 Script executed:
#!/bin/bash
# Verify mapping in the client
rg -nP --type=ts -C3 '\bgetOpenOrders\s*\('
rg -nP --type=ts -C3 '\bassetId\b|\bnextCursor\b'
rg -nP --type=ts -C3 '"asset_id"|\"next_cursor\"'Length of output: 19435
Camel-cased params require mapping at the client boundary
Ensure getOpenOrders translates assetId → asset_id and nextCursor → next_cursor (and removes undefined props) before calling the API — currently apiParams with assetId is passed straight to client.getOpenOrders. Location: src/actions/getActiveOrders.ts (apiParams creation L121–L126; client.getOpenOrders call L144).
🤖 Prompt for AI Agents
src/types.ts around lines 412-416 and src/actions/getActiveOrders.ts around
lines 121–126 and 144: getOpenOrders is passing camelCased apiParams (assetId,
nextCursor) straight to client.getOpenOrders; you must map assetId→asset_id and
nextCursor→next_cursor and strip any undefined props before calling the client.
Fix by creating a new params object for the client: if assetId is defined set
params.asset_id = assetId; if nextCursor is defined set params.next_cursor =
nextCursor; copy other required fields (e.g., market → market) only when
defined; do not pass undefined values (filter them out or conditionally assign)
and pass that snake_case, pruned object to client.getOpenOrders.
| _signTypedData: async (domain: any, types: any, value: any) => | ||
| wallet.signTypedData(domain, types, value), | ||
| getAddress: async () => wallet.address, |
There was a problem hiding this comment.
Ethers v5/v6 compatibility for typed‑data signing
wallet.signTypedData exists in v6; v5 uses _signTypedData. Current wrapper may break on v5. Make it version‑agnostic.
- _signTypedData: async (domain: any, types: any, value: any) =>
- wallet.signTypedData(domain, types, value),
+ _signTypedData: async (domain: any, types: any, value: any) => {
+ const w: any = wallet as any;
+ if (typeof w._signTypedData === 'function') {
+ return w._signTypedData(domain, types, value); // ethers v5
+ }
+ if (typeof w.signTypedData === 'function') {
+ return w.signTypedData(domain, types, value); // ethers v6
+ }
+ throw new Error('Wallet does not support EIP-712 signing');
+ },Apply the same change in initializeClobClientWithCreds.
Also applies to: 123-125
🤖 Prompt for AI Agents
In src/utils/clobClient.ts around lines 45-47 (and similarly lines 123-125 in
initializeClobClientWithCreds), the wrapper currently calls wallet.signTypedData
or _signTypedData directly which breaks compatibility between ethers v6
(signTypedData) and v5 (_signTypedData); change the wrapper to detect which
function exists at runtime and call the available one (e.g., use
wallet.signTypedData if defined, otherwise fall back to wallet._signTypedData),
forwarding domain/types/value and preserving async semantics; make the same
runtime existence check and fallback in the initializeClobClientWithCreds block
to ensure version-agnostic typed-data signing.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/actions/createApiKey.ts (2)
207-216: Secret leakage: logging raw API credentialsRaw response likely contains api_secret/passphrase. Don’t log secrets.
- logger.info( - '[createApiKeyAction] Raw API response:', - JSON.stringify(apiCredentials, null, 2) - ); + // Do not log raw credentials. Optionally log non-sensitive shape only. + logger.debug('[createApiKeyAction] Received API credentials response (fields masked)');
118-127: Auth signing: domain/types OK — make nonce unique and fix headers
- Domain and EIP‑712 types match Polymarket docs (name "ClobAuthDomain", version "1", chainId 137). L1 required headers: POLY_ADDRESS, POLY_SIGNATURE, POLY_TIMESTAMP, POLY_NONCE.
- Do not hardcode nonce = 0 — use a per-address unique or monotonically increasing nonce (or otherwise avoid reusing the same nonce/timestamp/signature) to prevent replay/nonce-check failures.
- For GET (derive-api-key) remove Content‑Type and add Accept; only set Content‑Type for requests with bodies (POST /auth/api-key).
- Locations: src/actions/createApiKey.ts — lines 118–127, 145–146, 152–166.
src/actions/getAllApiKeys.ts (1)
196-200: Secret leakage: logging raw responseDo not log full API key objects.
- logger.info( - '[getAllApiKeysAction] Raw response from ClobClient:', - JSON.stringify(apiKeysResponse, null, 2) - ); + logger.debug('[getAllApiKeysAction] Received API keys response (masked)');src/actions/placeOrder.ts (1)
451-456: Action name mismatch: exported name is "PLACE_ORDER" but you report "POLYMARKET_PLACE_ORDER"This breaks routing/metrics. Standardize on the actual action name.
- actions: ['POLYMARKET_PLACE_ORDER'], + actions: ['PLACE_ORDER'], @@ - actionName: 'POLYMARKET_PLACE_ORDER', + actionName: 'PLACE_ORDER',Also applies to: 476-483
src/actions/getOrderBookDepth.ts (1)
20-20: Action name is inconsistent with othersUse the POLYMARKET_ prefix for consistency with other actions and with examples referencing POLYMARKET_GET_ORDER_BOOK_DEPTH.
-export const getOrderBookDepthAction: Action = { - name: 'GET_ORDER_BOOK_DEPTH', +export const getOrderBookDepthAction: Action = { + name: 'POLYMARKET_GET_ORDER_BOOK_DEPTH',
♻️ Duplicate comments (2)
src/actions/retrieveAllMarkets.ts (1)
117-120: Type safety issue:next_cursorproperty not declared in MarketFilters.The code accesses
filterParams?.next_cursorbutfilterParamsis typed asMarketFilterswhich doesn't include this property. This could lead to TypeScript errors or runtime issues.Apply this fix to maintain type safety:
-let filterParams: MarketFilters = {}; +type MarketFiltersWithCursor = MarketFilters & { next_cursor?: string }; +let filterParams: MarketFiltersWithCursor = {};Or extract the cursor separately:
+const nextCursor = (filterParams as any).next_cursor ?? ''; const response = await (clobClient as any).getMarkets( - filterParams?.next_cursor || '', + nextCursor, filterParams );src/actions/getPriceHistory.ts (1)
96-96: API contract changed to ActionResult — audit callsites/testsEnsure no callers still expect boolean/Content. Update examples/README accordingly.
#!/bin/bash # Expect no boolean assertions on getPriceHistory results rg -nP -C2 'await\s+getPriceHistory\.handler' --type=ts rg -nP -C2 'expect\(\s*result\s*\)\.toBe\(' --type=ts
🧹 Nitpick comments (38)
src/actions/revokeApiKey.ts (1)
160-173: Consider consolidating error field redundancy.The ActionResult structure contains
error: trueinvaluesand both an error message indata.erroranderror: new Error(...)at the root level. This redundancy might cause confusion for consumers.Consider simplifying to:
return { text: errorMessage, values: { success: false, - error: true, }, data: { actionName: 'POLYMARKET_DELETE_API_KEY', error: 'No valid API key ID provided', }, success: false, error: new Error('No valid API key ID provided'), };src/actions/getOrderDetails.ts (1)
178-193: Consider simplifying the "order not found" response.The response includes both
success: falseandorderFound: falsein values, which is redundant. Consider using just theorderFoundfield for clarity.return { text: notFoundContent.text, values: { - success: false, orderFound: false, orderId, }, data: { actionName: 'POLYMARKET_GET_ORDER_DETAILS', error: 'Order not found', orderId, timestamp: new Date().toISOString(), }, success: false, error: new Error('Order not found'), };src/actions/getAccountAccessStatus.ts (1)
201-217: Consider adding error code classification.For better error handling by consumers, consider adding error codes or categories to distinguish between different failure types (auth failures, network issues, etc.).
Example enhancement:
const errorResult: ActionResult = { text: `❌ **Error**: ${errorMessage}`, values: { success: false, error: true, + errorCode: error instanceof Error && error.message.includes('401') ? 'AUTH_FAILED' : 'UNKNOWN_ERROR', }, data: { actionName: 'POLYMARKET_GET_ACCOUNT_ACCESS_STATUS', error: errorMessage, + errorCode: error instanceof Error && error.message.includes('401') ? 'AUTH_FAILED' : 'UNKNOWN_ERROR', timestamp: new Date().toISOString(), }, success: false, error: error instanceof Error ? error : new Error(String(error)), };src/actions/getSimplifiedMarkets.ts (4)
72-84: Avoid embedding Error objects in ActionResult (serialization/transport issues)Return strings in data/values; don’t attach native Error instances.
- success: false, - error: new Error(errorMessage), + success: false
197-216: Duped data between values and dataLarge payload (markets list) sits in both values and data. Keep counts in values; move arrays to data only.
- values: { - success: true, - marketCount: validMarkets.length, - totalCount: response.count || marketCount, - filteredCount: marketCount - validMarkets.length, - hasMoreResults: !!response.next_cursor && response.next_cursor !== 'LTE=', - }, + values: { + success: true, + marketCount: validMarkets.length, + totalCount: response.count || marketCount, + filteredCount: marketCount - validMarkets.length, + hasMoreResults: !!response.next_cursor && response.next_cursor !== 'LTE=', + }, data: { actionName: 'POLYMARKET_GET_SIMPLIFIED_MARKETS', - markets: validMarkets, + markets: validMarkets,
241-254: Error object in ActionResult (again)Same as above—drop the native Error instance.
- success: false, - error: error instanceof Error ? error : new Error(errorMessage), + success: false
175-177: Centralize cursor sentinel 'LTE=' into a shared constantsrc/types.ts documents "'LTE=' means the end" and the literal is used across many actions and tests (e.g., src/actions/getSimplifiedMarkets.ts:175-177,204-205; src/actions/retrieveAllMarkets.ts:154,182; many tests). Export a single NO_MORE_CURSOR (or helper hasMore(nextCursor)) and replace direct checks; update tests/fixtures to use the constant.
Suggested replacement:
- hasMoreResults: !!response.next_cursor && response.next_cursor !== 'LTE=', + // Use shared constant instead of hard-coded sentinel + hasMoreResults: Boolean(response.next_cursor && response.next_cursor !== NO_MORE_CURSOR),src/actions/getTradeHistory.ts (3)
219-237: Keep heavy arrays out of values; reserve values for scalarsMove trades array to data only; retain counts/cursor in values.
- values: { - success: true, - trades: tradesResponse.trades, - nextCursor: tradesResponse.next_cursor, - count: tradesResponse.count, - limit: tradesResponse.limit, - }, + values: { + success: true, + nextCursor: tradesResponse.next_cursor, + count: tradesResponse.count, + limit: tradesResponse.limit, + }, data: { actionName: 'POLYMARKET_GET_TRADE_HISTORY', ...apiParams, trades: tradesResponse.trades,
240-241: Callback payload consistencyOther actions pass actions alongside text/data. Standardize callback payload shape.
- if (callback) await callback({ text: responseResult.text, data: responseResult.data }); + if (callback) await callback({ text: responseResult.text, data: responseResult.data, actions: ['POLYMARKET_GET_TRADE_HISTORY'] });
245-259: Avoid Error objects in ActionResultReturn strings; log stack separately.
- success: false, - error: error instanceof Error ? error : new Error(String(error)), + success: falsesrc/actions/createApiKey.ts (3)
315-330: ActionResult contains full secrets — limit exposure surfacePrefer masking in values; if returning full secrets is required, keep them only in data and ensure downstream does not log data. Consider encrypting at rest.
- const successResult: ActionResult = { + const successResult: ActionResult = { text: successMessage, values: { success: true, - apiKey: responseData, + apiKey: { id: responseData.id }, // mask in values isNewKey, }, data: { actionName: 'CREATE_API_KEY', success: true, apiKey: responseData, isNewKey, }, success: true, };
334-337: Await the callbackKeep callback flow consistent and easier to trace.
- callback({ + await callback({ text: successMessage, data: successResult.data, });
360-373: Avoid Error objects in ActionResultReturn string state; log the actual error instance.
- success: false, - error: error instanceof Error ? error : new Error(String(error)), + success: falsesrc/actions/getAllApiKeys.ts (3)
118-126: Minor: source detection log is unreliableruntime.getSetting reads from both env and in-memory; the “runtime vs env” distinction here is misleading.
- apiKeySource: apiKey ? (runtime.getSetting('CLOB_API_KEY') ? 'runtime' : 'env') : 'none', + apiKeySource: apiKey ? 'available' : 'none',
168-173: Await the callback on error pathsKeep behavior consistent with success path.
- callback({ + await callback({ text: helpMessage, data: errorResult.data, });- callback({ + await callback({ text: errorMessage, data: errorResult.data, });Also applies to: 368-373
353-366: Avoid Error objects in ActionResultSame pattern—remove Error instance.
- success: false, - error: error instanceof Error ? error : new Error(String(error)), + success: falsesrc/actions/placeOrder.ts (4)
84-101: Unify callback payload shape and include actionsSome error paths send {text,data} without actions; others send full Content. Standardize to include actions consistently.
- await callback({ text: errorResult.text, data: errorResult.data }); + await callback({ text: errorResult.text, data: errorResult.data, actions: ['PLACE_ORDER'] });Also applies to: 208-212, 274-277, 300-303, 327-330, 360-363, 523-525
179-206: Avoid Error objects in ActionResultRemove native Error instances; keep strings in data/values.
- success: false, - error: new Error(errorMessage), + success: false(Apply similarly to other errorResult returns in this file.)
Also applies to: 309-326, 342-359, 489-520
240-247: Type assumptions: cast to ClobClient/anyConsider runtime guards to surface clearer errors if the client API changes (e.g., check typeof client.createOrder === 'function' before calling).
Also applies to: 336-337
151-167: Token/price/size regex fallback is permissiveMay capture unrelated 5+ digit numbers or prices from other phrases. Tighten patterns or require explicit labels in fallback.
- const tokenMatch = text.match(/(?:token|market|id)\s+([a-zA-Z0-9]+)|([0-9]{5,})/i); + const tokenMatch = text.match(/\b(?:token|market|id)\b[:\s]+([a-f0-9]{16,}|[1-9]\d{15,})/i);src/actions/setupWebsocket.ts (3)
118-121: Prefer a specific error type instead ofany.Line 118 catches with
error: any, but Line 120 accesseserror.messagewhich assumes Error type. Consider usingunknownand proper type narrowing.- } catch (error: any) { + } catch (error) { logger.warn( - `[setupWebsocketAction] LLM extraction failed or did not yield parameters (Error: ${error.message}). Attempting regex/manual extraction.` + `[setupWebsocketAction] LLM extraction failed or did not yield parameters (Error: ${error instanceof Error ? error.message : String(error)}). Attempting regex/manual extraction.` );
365-365: Loose typing on WebSocket message handler.The
dataparameter acceptsanytype, which bypasses TypeScript's type safety. Consider using a more specific type orunknownwith runtime validation.- wsClient.once('message', (data: any) => { + wsClient.once('message', (data: unknown) => {
371-393: Prefer a specific error type instead ofany.Similar to the earlier comment, use
unknownand proper type narrowing for better type safety.- } catch (error: any) { + } catch (error) { logger.error( '[setupWebsocketAction] Critical error during WebSocket setup (outside of event handlers):', error ); setActiveClobSocketClientReference(null); const errorContent = { - text: `❌ Critical WebSocket Setup Error: ${error.message}`, + text: `❌ Critical WebSocket Setup Error: ${error instanceof Error ? error.message : String(error)}`, }; if (callback) callback(errorContent); rejectPromise({ text: errorContent.text, values: { success: false, error: true, }, data: { actionName: 'POLYMARKET_SETUP_WEBSOCKET', - error: error.message, + error: error instanceof Error ? error.message : String(error), }, success: false, - error: error, + error: error instanceof Error ? error : new Error(String(error)), });src/actions/handleAuthentication.ts (1)
123-126: Potentially unsafe type assertion.The code uses
(clientSignerInstance as any).signer?.getAddress?.()which bypasses TypeScript's type checking. Consider defining proper types or interfaces.- const signerAddress = await (clientSignerInstance as any).signer?.getAddress?.(); + // If ClobClient has a signer property with getAddress method, consider extending the type definition + const signerAddress = 'signer' in clientSignerInstance && + typeof (clientSignerInstance as any).signer?.getAddress === 'function' + ? await (clientSignerInstance as any).signer.getAddress() + : undefined;src/actions/getClobMarkets.ts (1)
89-96: Document the type casting workaround more prominently.The cast to
anybypasses TypeScript's incomplete type definitions. Consider creating a proper type extension or filing an issue with the library maintainers.// NOTE: The TypeScript types for getMarkets only show next_cursor parameter, // but the actual API accepts additional filter parameters like category, active, limit. // We cast to any to bypass the incomplete type definition. + // TODO: Consider extending the ClobClient interface or contributing proper types upstream const marketsResponse = await (clobClient as any).getMarkets('', {Would you like me to create a TypeScript interface extension for the ClobClient to properly type these additional parameters?
src/actions/getPriceHistory.ts (3)
208-234: Latest/high/low computed from unsorted dataformatPriceHistoryResponse sorts by timestamp desc, but values.latestPrice uses the first element of the unsorted array. Derive stats from the same sorted series to avoid inconsistencies.
- const prices = priceHistory?.map((p: PricePoint) => p.p) || []; - const latestPrice = priceHistory?.length > 0 ? priceHistory[0].p : undefined; + const sorted = [...(priceHistory || [])].sort((a, b) => b.t - a.t); + const prices = sorted.map((p: PricePoint) => p.p); + const latestPrice = sorted.length > 0 ? sorted[0].p : undefined;
173-175: Validate and normalize interval against allowed setGuard against unsupported intervals and avoid leaking invalid values to the API.
-// Set default interval if not provided -const interval = params.interval || '1d'; +// Normalize interval +const allowed = new Set(['1m','5m','1h','1d','1w']); +const interval = allowed.has((params.interval || '1d')) ? (params.interval as string) : '1d';
195-206: Success callback: use data payload; consider passing sorted seriesAlign callback payload with other actions and reduce consumer surprises by passing the same sorted series used for stats/formatting.
- if (callback) { - await callback({ - text: responseMessage, - content: { - action: 'POLYMARKET_PRICE_HISTORY_RETRIEVED', - tokenId: params.tokenId, - interval: interval, - priceHistory: priceHistory, - pointsCount: priceHistory?.length || 0, - timestamp: new Date().toISOString(), - }, - }); - } + if (callback) { + await callback({ + text: responseMessage, + data: { + actionName: 'POLYMARKET_GET_PRICE_HISTORY', + tokenId: params.tokenId, + interval, + priceHistory: sorted, + pointsCount: sorted.length, + timestamp: new Date().toISOString(), + }, + }); + }src/actions/getActiveOrders.ts (1)
152-170: Type the client response to avoid lossy any-castsIf @polymarket/clob-client exposes types for OpenOrdersResponse (with data/next_cursor), prefer them to remove casts and conditional shape checks.
Would you like a follow‑up diff replacing any with proper types once we confirm the library’s response shape?
src/actions/getOpenMarkets.ts (2)
85-106: Category param is parsed but never appliedThe LLM may return category, but it’s unused. Either filter client results or pass a category filter to the API.
- const marketsResponse = await (clobClient as any).getMarkets('', { - active: true, // Only active markets - limit: params.limit || 50, // Get more to sort and filter - }); + const marketsResponse = await (clobClient as any).getMarkets('', { + active: true, + limit: params.limit || 50, + // If the API supports category, forward it; otherwise filter locally + ...(params.category ? { category: params.category } : {}), + }); const allMarkets = marketsResponse.data || []; + const categoryFiltered = params.category + ? allMarkets.filter( + (m: any) => (m.category || '').toLowerCase() === params.category!.toLowerCase() + ) + : allMarkets; - // Filter for truly open markets (active=true AND closed=false) + // Filter for truly open markets (active=true AND closed=false) - const openMarkets = allMarkets.filter( + const openMarkets = categoryFiltered.filter( (market) => market.active === true && market.closed === false );
297-303: Yes/No labeling can mismatch pricesYou compute yesPrice/noPrice by outcome search but label with tokens[0]/tokens[1]. Use the matched tokens’ outcomes instead.
- if (tokens.length >= 2) { - const yesPrice = - tokens.find((t) => t.outcome?.toLowerCase().includes('yes'))?.price || '0.50'; - const noPrice = tokens.find((t) => t.outcome?.toLowerCase().includes('no'))?.price || '0.50'; - response += `├─ Tokens: ${tokens[0]?.outcome || 'Yes'} (${yesPrice}) | ${tokens[1]?.outcome || 'No'} (${noPrice})\n`; - } + if (tokens.length >= 2) { + const yesToken = tokens.find((t) => t.outcome?.toLowerCase().includes('yes')); + const noToken = tokens.find((t) => t.outcome?.toLowerCase().includes('no')); + const yesLabel = yesToken?.outcome || 'Yes'; + const noLabel = noToken?.outcome || 'No'; + const yesPrice = yesToken?.price ?? '0.50'; + const noPrice = noToken?.price ?? '0.50'; + response += `├─ Tokens: ${yesLabel} (${yesPrice}) | ${noLabel} (${noPrice})\n`; + }src/actions/getOrderBookDepth.ts (2)
248-258: Best bid/ask may not be at index 0Unless the API guarantees sorted levels, compute best bid as max(bids.price) and best ask as min(asks.price).
- const bestBid = bidCount > 0 ? orderBook.bids[0] : null; - const bestAsk = askCount > 0 ? orderBook.asks[0] : null; + const bestBid = + bidCount > 0 + ? orderBook.bids.reduce((a, b) => (+a.price >= +b.price ? a : b)) + : null; + const bestAsk = + askCount > 0 + ? orderBook.asks.reduce((a, b) => (+a.price <= +b.price ? a : b)) + : null;
221-226: Avoid any-cast if possibleIf the client expects { token_id: string }[], add a local type to keep compile‑time guarantees rather than casting.
-const bookParams = tokenIds.map((tokenId) => ({ token_id: tokenId })); -const orderBooks: OrderBook[] = await clobClient.getOrderBooks(bookParams as any); +type BookParam = { token_id: string }; +const bookParams: BookParam[] = tokenIds.map((tokenId) => ({ token_id: tokenId })); +const orderBooks: OrderBook[] = await clobClient.getOrderBooks(bookParams as unknown as any);src/actions/getMidpointPrice.ts (1)
124-126: Token extraction regex allows non‑numeric IDsIf tokens are strictly numeric in your context, restrict to digits to reduce false positives; otherwise, keep as‑is.
-const tokenMatch = text.match(/(?:token|market|id)\s+([a-zA-Z0-9]+)|([0-9]{5,})/i); +const tokenMatch = text.match(/(?:token|market|id)\s+(\d+)|(\d{5,})/i);src/actions/getSamplingMarkets.ts (1)
206-206: Preferas const satisfiesfor better type safety.Using
satisfiesprovides type checking but loses literal type inference. Consider usingas const satisfiesfor stricter type safety.- ] satisfies ActionExample[][], + ] as const satisfies ActionExample[][],src/actions/handleRealtimeUpdates.ts (3)
55-55: WebSocket message handler uses relaxed typing.The message handler accepts
rawData: any. Consider using a more specific type likeBuffer | ArrayBuffer | Buffer[]which are the actual types that ws emits.- wsClient.on('message', (rawData: any) => { + wsClient.on('message', (rawData: Buffer | ArrayBuffer | Buffer[]) => {
100-100: Error logging only captures message property.Logging only
error.messagemay lose important debugging context. Consider logging the full error object in development mode or structured logging.- logger.error('[WS Event] Error:', error.message); + logger.error('[WS Event] Error:', error);
34-38: Consider adding type safety to PolymarketWebSocketEvent.The interface uses
data: anyand index signature[key: string]: any. Consider defining more specific types based on actual Polymarket WebSocket events for better type safety.Based on the comment mentioning event types like 'book_update', 'order_update', 'trade', consider:
-interface PolymarketWebSocketEvent { - type: string; // e.g., 'book_update', 'order_update', 'trade', etc. - data: any; - [key: string]: any; -} +type PolymarketEventType = 'book_update' | 'order_update' | 'trade' | string; + +interface PolymarketWebSocketEvent { + type: PolymarketEventType; + event?: string; + data: Record<string, unknown>; + timestamp?: string; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (24)
src/actions/checkOrderScoring.ts(4 hunks)src/actions/createApiKey.ts(7 hunks)src/actions/getAccountAccessStatus.ts(3 hunks)src/actions/getActiveOrders.ts(4 hunks)src/actions/getAllApiKeys.ts(6 hunks)src/actions/getBestPrice.ts(5 hunks)src/actions/getClobMarkets.ts(5 hunks)src/actions/getMarketDetails.ts(6 hunks)src/actions/getMidpointPrice.ts(5 hunks)src/actions/getOpenMarkets.ts(5 hunks)src/actions/getOrderBookDepth.ts(8 hunks)src/actions/getOrderBookSummary.ts(8 hunks)src/actions/getOrderDetails.ts(7 hunks)src/actions/getPriceHistory.ts(7 hunks)src/actions/getSamplingMarkets.ts(5 hunks)src/actions/getSimplifiedMarkets.ts(5 hunks)src/actions/getSpread.ts(5 hunks)src/actions/getTradeHistory.ts(3 hunks)src/actions/handleAuthentication.ts(6 hunks)src/actions/handleRealtimeUpdates.ts(5 hunks)src/actions/placeOrder.ts(8 hunks)src/actions/retrieveAllMarkets.ts(6 hunks)src/actions/revokeApiKey.ts(6 hunks)src/actions/setupWebsocket.ts(12 hunks)
🔇 Additional comments (31)
src/actions/revokeApiKey.ts (2)
1-9: LGTM! Proper imports for ActionResult migration.The imports correctly include the necessary
ActionandActionResulttypes for the handler's new return type structure.
285-297: LGTM! Clean success response structure.The success ActionResult is well-structured with clear values and data payloads, properly distinguishing between summary values and detailed data.
src/actions/getOrderDetails.ts (2)
124-136: Consistent ActionResult error handling.Good implementation of the ActionResult error pattern, maintaining consistency with the callback mechanism while providing structured error details.
220-239: LGTM! Comprehensive success response.Excellent structure with detailed field mapping in values for quick access and complete order object in data for full details.
src/actions/retrieveAllMarkets.ts (1)
176-194: LGTM! Well-structured market data response.The success ActionResult properly separates summary metrics in values from detailed market data in the data field, with clear pagination support.
src/actions/getOrderBookSummary.ts (2)
52-58: Handler return type properly migrated to ActionResult.The change from
Promise<Content>toPromise<ActionResult>aligns with the PR-wide migration pattern.
316-345: Excellent success response structure with comprehensive market data.The ActionResult provides well-organized values for quick access to key metrics (spreads, counts, prices) while preserving complete order book data in the data field. The summary statistics are particularly useful.
src/actions/getAccountAccessStatus.ts (1)
172-196: LGTM! Comprehensive access status response.The ActionResult structure effectively captures certification requirements, managed API keys, and session key information with proper separation between values and detailed data.
src/actions/getTradeHistory.ts (1)
128-129: API contract changed to Promise — confirm callers/tests and core typesgetTradeHistoryAction.handler (and most other actions) already use ): Promise. Tests and some callers still treat handler results as Content — verify @elizaos/core's Action/ActionResult shape is compatible with Content or update callers/tests to use ActionResult.
Key spots to update/check:
- src/actions/getTradeHistory.ts (handler signature)
- Tests casting/expecting Content: tests/getMidpointPrice.test.ts, tests/getOrderBookDepth.test.ts, many tests/*.test.ts files
- e2e/starter-plugin.test.ts (direct action.handler invocation)
src/actions/getSimplifiedMarkets.ts (1)
55-55: No changes required — Action.handler already returns Promise and callers/tests expect ActionResult.Verified: src/actions/getSimplifiedMarkets.ts declares handler as Promise and returns ActionResult-shaped objects; tests in src/tests/getSimplifiedMarkets.test.ts await the handler and assert on result.text, result.data, and result.actions. Other actions in src/actions follow the same Promise pattern.
src/actions/getBestPrice.ts (1)
1-310: LGTM! Clean migration to ActionResult pattern.The handler properly returns structured ActionResult objects with appropriate success/error states and maintains backward compatibility through the callback mechanism.
src/actions/getSpread.ts (1)
1-257: LGTM! Consistent ActionResult implementation.The file correctly implements the ActionResult pattern with proper error handling, LLM extraction with regex fallback, and well-structured response payloads.
src/actions/handleAuthentication.ts (1)
269-270: Fix variable name inconsistency in error handler.The error handler references
errorResultin the callback, but the variable defined iserrorResult. This appears to be correct, but ensure consistency throughout.src/actions/getActiveOrders.ts (2)
202-223: LGTM: ActionResult payload and callback shape are consistentStructured values/data, timestamp, and success flags look good.
191-194: Magic sentinel ('LTE=') is documented and intentionally used.Definition exists in src/types.ts ("'LTE=' means the end") and the sentinel is referenced across actions and tests (e.g., src/actions/getSamplingMarkets.ts, retrieveAllMarkets.ts, getTradeHistory.ts, getClobMarkets.ts and many tests). No change required.
src/actions/getOpenMarkets.ts (2)
160-181: LGTM: ActionResult success payload and callback usage are consistentGood structure and timestamps; mirrors the rest of the migration.
139-148: Prefer a canonical listing/created timestamp or explicitly document this heuristicsrc/types.ts defines Market with end_date_iso and game_start_time — there is no created_at/start_date field. Since a true “listing/created” timestamp isn’t available, either (A) map/use the API’s createdAt/startDate and sort by that, or (B) keep sorting by end_date_iso but add a short code comment and tests documenting that end_date_iso is being used as a heuristic for “listing time” (see src/actions/getOpenMarkets.ts lines ~139–148).
src/actions/getMidpointPrice.ts (2)
176-194: LGTM: ActionResult payload, values/data, and callback are consistentGood structure with timestamps and clear success flags.
70-93: Config validation path matches project conventionEarly return with structured error and callback is consistent with other actions.
src/actions/getSamplingMarkets.ts (4)
3-3: LGTM! Clean migration to ActionResult pattern.The type imports have been properly updated to use ActionResult and ActionExample as a type-only import, which aligns with the PR-wide migration pattern.
Also applies to: 9-9
55-59: Handler signature properly updated to ActionResult.The return type change from
Promise<boolean>toPromise<ActionResult>is correctly implemented, maintaining consistency with the broader PR changes.
111-127: Well-structured success response with comprehensive data.The ActionResult success payload includes all necessary fields: text, values (with success flag and metadata), data (with actionName and timestamp), and a top-level success flag. This provides excellent observability.
149-162: Comprehensive error handling with structured response.The error path correctly returns an ActionResult with appropriate error flags and includes the error object. The error message provides helpful troubleshooting steps.
src/actions/checkOrderScoring.ts (3)
113-127: Properly structured error result with clear user guidance.The error handling provides a well-formatted ActionResult with appropriate error flags and helpful user instructions for providing Order IDs.
156-173: Success response includes comprehensive scoring information.The success ActionResult properly includes the scoring response data, order IDs, and formatted text output. The data structure is consistent with other actions.
180-196: Error handling maintains consistency with success path.The catch block error handling properly constructs an ActionResult with appropriate error information and maintains the same structure as the success path.
src/actions/getMarketDetails.ts (3)
61-78: Configuration error properly handled with ActionResult.The missing CLOB_API_URL configuration check correctly returns an ActionResult with error details instead of throwing, which improves error recovery.
223-237: Comprehensive market details in success response.The success ActionResult includes the complete market object in values and data, along with the conditionId for traceability. The formatted text provides excellent detail.
251-278: Error response includes helpful debugging context.The error ActionResult includes the condition ID that failed, which aids in debugging. The error message provides a clear checklist for troubleshooting.
src/actions/handleRealtimeUpdates.ts (2)
165-179: Missing WebSocket client properly handled.The check for missing activeWebSocketClient correctly returns an ActionResult with appropriate error information and user guidance.
198-213: Success response includes WebSocket state information.The success ActionResult includes the WebSocket readyState, which is valuable for debugging connection issues. Good inclusion of timestamp for event correlation.
| logger.info( | ||
| '[createApiKeyAction] Extracted fields:', | ||
| JSON.stringify({ | ||
| id: responseData.id, | ||
| secretLength: responseData.secret?.length, | ||
| passphraseLength: responseData.passphrase?.length, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Secret leakage: “Extracted fields” log shows secret/passphrase lengths
Even lengths can be sensitive. Remove or gate behind a very verbose debug flag and avoid per-request logs.
- logger.info(
- '[createApiKeyAction] Extracted fields:',
- JSON.stringify({
- id: responseData.id,
- secretLength: responseData.secret?.length,
- passphraseLength: responseData.passphrase?.length,
- })
- );
+ logger.debug('[createApiKeyAction] Extracted credential ids (secrets masked)');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.info( | |
| '[createApiKeyAction] Extracted fields:', | |
| JSON.stringify({ | |
| id: responseData.id, | |
| secretLength: responseData.secret?.length, | |
| passphraseLength: responseData.passphrase?.length, | |
| }) | |
| ); | |
| logger.debug('[createApiKeyAction] Extracted credential ids (secrets masked)'); |
🤖 Prompt for AI Agents
In src/actions/createApiKey.ts around lines 239 to 246, the logger.info call
emits secret/passphrase lengths which can leak sensitive information; remove
that per-request logging or gate it behind a very verbose debug flag. Replace
the info log with a non-sensitive message (e.g., that key creation succeeded
with id only) or wrap the existing detailed log in a conditional that checks a
secure debug-level flag (not enabled in production) before logging lengths;
ensure you do not log secrets or lengths by default and prefer aggregated
telemetry if you need usage metrics.
|
|
||
| if (callback) await callback(responseContent); | ||
| return responseContent; | ||
| if (callback) await callback({ text: responseResult.text, data: responseResult.data }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Inconsistent callback payload structure.
The callback receives { text, data } while other actions in the codebase might expect different structures. Verify consistency across all actions.
Also applies to: 216-216
🏁 Script executed:
#!/bin/bash
# Check callback patterns across all action files
echo "Analyzing callback patterns in action handlers..."
rg -A 2 "callback\(\{" src/actions/ --type ts | head -30
# Check if there's a consistent pattern
echo -e "\nChecking for different callback patterns..."
rg "callback\(" src/actions/ --type ts | grep -E "text:|data:|action:" | sort | uniq -c | sort -rnLength of output: 5885
Unify callback payload across actions.
- Observed: most actions call callback({ text, data }); exceptions: src/actions/setupWebsocket.ts uses callback({ text:
❌ ...}) and src/actions/revokeApiKey.ts uses callback({ text, action }). - Action: define a shared CallbackPayload type (e.g. { text: string; data?: unknown; action?: string }) and update these handlers and all callers to use it.
🤖 Prompt for AI Agents
In src/actions/getAccountAccessStatus.ts around line 195, the callback
invocation uses a payload shape that must be unified with the new shared
CallbackPayload type; update this file to call callback with an object matching
the shared type (e.g. { text: string; data?: unknown; action?: string }) —
change the current await callback({ text: responseResult.text, data:
responseResult.data }) to use that shape, import or reference the new
CallbackPayload type, and ensure any optional fields (data/action) are present
only when relevant; then update all related action files and their callers
(including setupWebsocket.ts and revokeApiKey.ts) to accept and emit the same
CallbackPayload so the payload is consistent across actions.
| @@ -1,4 +1,4 @@ | |||
| import { IAgentRuntime, Memory, State, HandlerCallback, logger } from '@elizaos/core'; | |||
| import { IAgentRuntime, Memory, State, HandlerCallback, logger, ActionResult } from '@elizaos/core'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing Action typing on exported action
Annotate with Action for compile-time guarantees and API consistency.
-import { IAgentRuntime, Memory, State, HandlerCallback, logger, ActionResult } from '@elizaos/core';
+import { IAgentRuntime, Memory, State, HandlerCallback, logger, ActionResult, Action } from '@elizaos/core';
@@
-export const getAllApiKeysAction = {
+export const getAllApiKeysAction: Action = {Also applies to: 25-26
🤖 Prompt for AI Agents
In src/actions/getAllApiKeys.ts around lines 1 and 25-26, the exported action is
missing the Action type annotation; import the Action type from '@elizaos/core'
(add it to the existing import list) and annotate the exported constant/function
with the appropriate Action generic (e.g., Action<IAgentRuntime, Memory, State,
HandlerCallback, ActionResult> or the project-specific generics) so the export
has compile-time typing and matches other actions.
| text: errorMessage, | ||
| actions: ['GET_ORDER_BOOK_DEPTH'], | ||
| data: { error: errorMessage }, | ||
| values: { | ||
| success: false, | ||
| error: true, | ||
| }, | ||
| data: { | ||
| actionName: 'GET_ORDER_BOOK_DEPTH', | ||
| error: errorMessage, | ||
| }, | ||
| success: false, | ||
| error: new Error(errorMessage), | ||
| }; |
There was a problem hiding this comment.
Normalize actionName across all results
Some paths use GET_ORDER_BOOK_DEPTH while others use POLYMARKET_GET_ORDER_BOOK_DEPTH. Standardize to the prefixed name.
- actionName: 'GET_ORDER_BOOK_DEPTH',
+ actionName: 'POLYMARKET_GET_ORDER_BOOK_DEPTH',Apply to all error/result constructions in this file.
Also applies to: 108-113, 151-156, 290-303
🤖 Prompt for AI Agents
In src/actions/getOrderBookDepth.ts around lines 61-72 (and also update the
similar blocks at 108-113, 151-156, and 290-303), the actionName field uses
'GET_ORDER_BOOK_DEPTH' in some result/error objects; change all occurrences to
the standardized prefixed value 'POLYMARKET_GET_ORDER_BOOK_DEPTH' so every
success and error result in this file uses the same actionName string.
| return { | ||
| text: errorContent.text, | ||
| values: { | ||
| success: false, | ||
| error: true, | ||
| tokenId, | ||
| }, | ||
| data: { | ||
| actionName: 'POLYMARKET_GET_ORDER_BOOK', | ||
| error: errorMessage, | ||
| tokenId, | ||
| timestamp: new Date().toISOString(), | ||
| }, | ||
| success: false, | ||
| error: error instanceof Error ? error : new Error(errorMessage), | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify if tests and documentation need updates for the new return type.
The handler now returns Promise<ActionResult> instead of Promise<Content>. Ensure that all tests and documentation are updated accordingly.
🏁 Script executed:
#!/bin/bash
# Check for test files that might need updates for getOrderBookSummaryAction
echo "Searching for test files related to getOrderBookSummary..."
fd -e test.ts -e test.js | xargs rg -l "getOrderBookSummary" 2>/dev/null || true
# Check for any documentation that references the handler
echo -e "\nSearching for documentation references..."
fd -e md -e mdx | xargs rg -l "getOrderBookSummary" 2>/dev/null || true
# Check for any direct handler calls that might expect Content
echo -e "\nSearching for direct handler calls..."
rg -n "getOrderBookSummaryAction\.handler" --type ts --type jsLength of output: 2045
Update tests and README for new Promise return type.
tests/getOrderBookSummary.test.ts (calls ~lines 119–417) and README.md still assume Promise; update tests' assertions/mocks and types to expect the ActionResult shape (text, values {success, error, tokenId}, data {actionName, error, tokenId, timestamp}, success, error).
| const errorMessage = 'Failed to extract token ID from message. Please specify a token ID.'; | ||
| if (callback) { | ||
| await callback({ | ||
| text: `❌ **Error**: ${errorMessage}`, | ||
| content: { | ||
| action: 'POLYMARKET_PRICE_HISTORY_ERROR', | ||
| error: errorMessage, | ||
| }, | ||
| }); | ||
| } | ||
| return { | ||
| text: errorMessage, | ||
| values: { | ||
| success: false, | ||
| error: true, | ||
| }, | ||
| data: { | ||
| actionName: 'POLYMARKET_GET_PRICE_HISTORY', | ||
| error: errorMessage, | ||
| }, | ||
| success: false, | ||
| error: new Error(errorMessage), | ||
| }; |
There was a problem hiding this comment.
Unify callback payload shape: use { text, data } instead of { text, content }
Other actions in this PR now pass callback({ text, data }). This file still uses content with action strings, which will break consumers expecting the new shape.
Apply this diff to align the callback contract in the LLM‑extraction error path:
- if (callback) {
- await callback({
- text: `❌ **Error**: ${errorMessage}`,
- content: {
- action: 'POLYMARKET_PRICE_HISTORY_ERROR',
- error: errorMessage,
- },
- });
- }
+ if (callback) {
+ await callback({
+ text: `❌ **Error**: ${errorMessage}`,
+ data: {
+ actionName: 'POLYMARKET_GET_PRICE_HISTORY',
+ error: errorMessage,
+ timestamp: new Date().toISOString(),
+ },
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const errorMessage = 'Failed to extract token ID from message. Please specify a token ID.'; | |
| if (callback) { | |
| await callback({ | |
| text: `❌ **Error**: ${errorMessage}`, | |
| content: { | |
| action: 'POLYMARKET_PRICE_HISTORY_ERROR', | |
| error: errorMessage, | |
| }, | |
| }); | |
| } | |
| return { | |
| text: errorMessage, | |
| values: { | |
| success: false, | |
| error: true, | |
| }, | |
| data: { | |
| actionName: 'POLYMARKET_GET_PRICE_HISTORY', | |
| error: errorMessage, | |
| }, | |
| success: false, | |
| error: new Error(errorMessage), | |
| }; | |
| const errorMessage = 'Failed to extract token ID from message. Please specify a token ID.'; | |
| if (callback) { | |
| await callback({ | |
| text: `❌ **Error**: ${errorMessage}`, | |
| data: { | |
| actionName: 'POLYMARKET_GET_PRICE_HISTORY', | |
| error: errorMessage, | |
| timestamp: new Date().toISOString(), | |
| }, | |
| }); | |
| } | |
| return { | |
| text: errorMessage, | |
| values: { | |
| success: false, | |
| error: true, | |
| }, | |
| data: { | |
| actionName: 'POLYMARKET_GET_PRICE_HISTORY', | |
| error: errorMessage, | |
| }, | |
| success: false, | |
| error: new Error(errorMessage), | |
| }; |
🤖 Prompt for AI Agents
In src/actions/getPriceHistory.ts around lines 121 to 143, the callback payload
in the LLM-extraction error path uses { text, content } while the rest of the
codebase expects { text, data }; update the callback call to use { text, data }
and move the existing content payload into data (e.g., data: { action:
'POLYMARKET_PRICE_HISTORY_ERROR', error: errorMessage }) so the callback shape
matches other actions and consumers receive the action and error fields under
data.
| const errorMessage = 'Token ID is required for price history retrieval'; | ||
| if (callback) { | ||
| await callback({ | ||
| text: `❌ **Error**: ${errorMessage}`, | ||
| content: { | ||
| action: 'POLYMARKET_PRICE_HISTORY_ERROR', | ||
| error: errorMessage, | ||
| }, | ||
| }); | ||
| } | ||
| return { | ||
| text: errorMessage, | ||
| values: { | ||
| success: false, | ||
| error: true, | ||
| }, | ||
| data: { | ||
| actionName: 'POLYMARKET_GET_PRICE_HISTORY', | ||
| error: errorMessage, | ||
| }, | ||
| success: false, | ||
| error: new Error(errorMessage), | ||
| }; |
There was a problem hiding this comment.
Same issue: callback should pass data, not content
Mirror the repo‑wide convention here too.
- if (callback) {
- await callback({
- text: `❌ **Error**: ${errorMessage}`,
- content: {
- action: 'POLYMARKET_PRICE_HISTORY_ERROR',
- error: errorMessage,
- },
- });
- }
+ if (callback) {
+ await callback({
+ text: `❌ **Error**: ${errorMessage}`,
+ data: {
+ actionName: 'POLYMARKET_GET_PRICE_HISTORY',
+ error: errorMessage,
+ timestamp: new Date().toISOString(),
+ },
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const errorMessage = 'Token ID is required for price history retrieval'; | |
| if (callback) { | |
| await callback({ | |
| text: `❌ **Error**: ${errorMessage}`, | |
| content: { | |
| action: 'POLYMARKET_PRICE_HISTORY_ERROR', | |
| error: errorMessage, | |
| }, | |
| }); | |
| } | |
| return { | |
| text: errorMessage, | |
| values: { | |
| success: false, | |
| error: true, | |
| }, | |
| data: { | |
| actionName: 'POLYMARKET_GET_PRICE_HISTORY', | |
| error: errorMessage, | |
| }, | |
| success: false, | |
| error: new Error(errorMessage), | |
| }; | |
| const errorMessage = 'Token ID is required for price history retrieval'; | |
| if (callback) { | |
| await callback({ | |
| text: `❌ **Error**: ${errorMessage}`, | |
| data: { | |
| actionName: 'POLYMARKET_GET_PRICE_HISTORY', | |
| error: errorMessage, | |
| timestamp: new Date().toISOString(), | |
| }, | |
| }); | |
| } | |
| return { | |
| text: errorMessage, | |
| values: { | |
| success: false, | |
| error: true, | |
| }, | |
| data: { | |
| actionName: 'POLYMARKET_GET_PRICE_HISTORY', | |
| error: errorMessage, | |
| }, | |
| success: false, | |
| error: new Error(errorMessage), | |
| }; |
🤖 Prompt for AI Agents
In src/actions/getPriceHistory.ts around lines 148 to 170, the callback
currently sends a payload with a "content" key instead of the repo‑standard
"data" key; update the callback invocation to pass the same object under "data"
(e.g., data: { action: 'POLYMARKET_PRICE_HISTORY_ERROR', error: errorMessage })
while keeping the text field unchanged so the callback payload matches the
repository convention.
| const priceHistory = await (clobClient as any).getPricesHistory({ | ||
| token_id: params.tokenId, | ||
| interval: interval as any, | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify CLOB client method and params
Double‑check that the client really exposes getPricesHistory({ token_id, interval }) (spelling and shape). A mismatch will 500 at runtime.
🏁 Script executed:
#!/bin/bash
# Locate CLOB client method definition/usages to confirm signature
rg -nP --type=ts -C2 '\bgetPricesHistory\b|getPriceHistory\b'
rg -nP --type=ts -C2 'initializeClobClient\('Length of output: 17820
Fix CLOB client call signature — use positional args
The Clob client expects getPricesHistory(tokenId, interval) (positional). Current code passes an object ({ token_id, interval }) which mismatches and will fail — change to:
await (clobClient as any).getPricesHistory(params.tokenId, interval);
Location: src/actions/getPriceHistory.ts:180-183 — tests (tests/getPriceHistory.test.ts) assert positional args.
🤖 Prompt for AI Agents
In src/actions/getPriceHistory.ts around lines 180 to 183, the CLOB client's
getPricesHistory is being called with a single object ({ token_id, interval })
but the client expects positional arguments; change the call to pass
params.tokenId as the first arg and interval as the second (await (clobClient as
any).getPricesHistory(params.tokenId, interval)); ensure naming matches the
positional API and update any related typings if needed.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores