Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Playwright + LLM "Browser Agent" and wires optional prompt/LLM metadata (promptInstructions, promptLlm*) through robot creation, execution (scheduler/worker/record), APIs, and frontend UI; agent outputs are captured as Changes
Sequence DiagramsequenceDiagram
participant UI as Frontend (RobotCreate)
participant API as Backend API
participant Scheduler as Scheduler/Worker
participant Agent as Browser Agent
participant LLM as LLM Provider
participant Page as Playwright Page
UI->>API: POST create scrape robot (promptInstructions, promptLlm*)
API-->>Scheduler: enqueue/run scrape (includes interpreterSettings.promptInstructions/formats)
Note over Scheduler,Agent: If promptInstructions present and currentPage available
Scheduler->>Agent: executeBrowserAgent(currentPage, promptInstructions, llmConfig)
Agent->>Page: Capture state (screenshot, text, elements, URL/title)
Agent->>LLM: Direct-answer attempt (raw-text)
alt LLM returns ANSWER
Agent-->>Scheduler: Return result (steps: [])
else Needs interaction
loop up to 20
Agent->>LLM: Send state + history (multimodal/chat)
LLM-->>Agent: JSON action response
Agent->>Page: Execute action (click/type/scroll/...)
Agent->>Page: Wait for networkidle & delay
Agent-->>Agent: Record step
end
Agent-->>Scheduler: Return final result + steps
end
Scheduler->>Scheduler: Persist serializableOutput.promptResult = [{content, steps}]
Scheduler-->>API: Save run output
API-->>UI: Respond with run data (includes promptResult)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[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 (7)
server/src/pgboss-worker.ts (2)
312-369:⚠️ Potential issue | 🟠 MajorReturn SmartQuery results through webhooks too.
The result is saved in
serializableOutput.smartQuery, but therun_completedwebhook payload only includes markdown/html/screenshots. Add the SmartQuery payload so async consumers can receive the feature output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/pgboss-worker.ts` around lines 312 - 369, The webhook payload for run completion is missing the SmartQuery results; update the code that builds webhookPayload (before the sendWebhook call in the markdown robot completion block) to add serializableOutput.smartQuery when present (e.g., if (serializableOutput.smartQuery) webhookPayload.smartQuery = serializableOutput.smartQuery), ensuring the same SmartQuery value stored earlier is forwarded in the 'run_completed' webhook sent by sendWebhook.
300-327:⚠️ Potential issue | 🟠 MajorRe-check abort status before marking the run successful.
SmartQuery can run for a long time; if the user aborts during it, Line 321 can still overwrite the run as
success. Check the latest run status after SmartQuery and before the success update.🐛 Suggested fix
if (promptInstructions) { try { @@ } } + + const latestRun = await Run.findOne({ where: { runId: data.runId } }); + if (latestRun && ['aborted', 'aborting'].includes(latestRun.status)) { + logger.log('info', `Run ${data.runId} was aborted during smart query, skipping success update`); + await destroyRemoteBrowser(browserId, data.userId); + return { success: true }; + } // Success update await run.update({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/pgboss-worker.ts` around lines 300 - 327, After executing the long-running SmartQuery (promptInstructions and executeBrowserAgent), re-check the current run status before calling run.update(... success ...): reload or re-fetch the run record (e.g., run.reload() or query by run id) to ensure it wasn't aborted during SmartQuery, and only proceed to mark status: 'success' and set finishedAt/log/outputs when the latest run.status is still not 'aborted' (or other terminal aborted state); if it is aborted, skip the success update and instead leave the aborted status/log intact or write a resumable message into serializableOutput.server/src/api/record.ts (1)
803-828:⚠️ Potential issue | 🟠 MajorExpose SmartQuery results to API callers and webhooks.
This stores
serializableOutput.smartQuery, but the response formatter and webhook payload do not include it. The run can succeed while the new result is effectively hidden from API/webhook consumers.🐛 Suggested fix
if (serializableOutput.markdown) webhookPayload.markdown = markdown; if (serializableOutput.html) webhookPayload.html = html; + if (serializableOutput.smartQuery) webhookPayload.smartQuery = serializableOutput.smartQuery; if (uploadedBinaryOutput['screenshot-visible']) webhookPayload.screenshot_visible = uploadedBinaryOutput['screenshot-visible'];Also add
smartQuerytoformatRunResponse(...)so/api/robots/:id/runsand run-start responses include it.Also applies to: 859-875
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/record.ts` around lines 803 - 828, The smartQuery result is written into serializableOutput.smartQuery by executeBrowserAgent but not exposed in API/webhook responses; update the response formatter and webhook payload to include it by adding smartQuery to the object returned by formatRunResponse (and any similar formatter used for run-start/run-complete payloads) so that run.update persists and API endpoints (/api/robots/:id/runs) and webhook builders include serializableOutput.smartQuery in their output; verify any serialization or typing for formatRunResponse, run-start response builders, and webhook payload constructors to accept and forward the smartQuery field.server/src/workflow-management/scheduler/index.ts (1)
328-400:⚠️ Potential issue | 🟠 MajorInclude SmartQuery output in scheduled-run webhooks.
serializableOutput.smartQueryis persisted, but the scheduled scrape webhook never includes it, so webhook consumers do not receive the result this feature produced.🐛 Suggested fix
if (formats.includes('markdown')) webhookPayload.markdown = markdown; if (formats.includes('html')) webhookPayload.html = html; + if (serializableOutput.smartQuery) webhookPayload.smartQuery = serializableOutput.smartQuery; if (uploadedBinaryOutput['screenshot-visible']) webhookPayload.screenshot_visible = uploadedBinaryOutput['screenshot-visible'];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/workflow-management/scheduler/index.ts` around lines 328 - 400, The webhookPayload currently omits the SmartQuery result even though serializableOutput.smartQuery is saved; update the code that builds webhookPayload (the webhookPayload object near where serializableOutput and uploadedBinaryOutput are available) to add the smart-query data—e.g., if serializableOutput.smartQuery exists, set webhookPayload.smart_query = serializableOutput.smartQuery (or webhookPayload.smartQuery if you prefer camelCase to match other consumers) so scheduled-run webhooks include the SmartQuery output before the webhook is sent.server/src/api/sdk.ts (1)
140-159:⚠️ Potential issue | 🟠 MajorNormalize SDK SmartQuery metadata before saving.
smartQueries/smart_queriescan be non-strings, but the executor later calls string methods onpromptInstructions. Mirror the storage route: coerce safely, cap length, validate provider, and avoid storing raw API keys in metadata.🐛 Suggested fix
- const promptInstructionsForMeta = type === 'scrape' - ? ((workflowFile.meta as any).promptInstructions || (workflowFile.meta as any).smartQueries || (workflowFile.meta as any).smart_queries) as string | undefined - : undefined; + const rawPromptInstructions = type === 'scrape' + ? ((workflowFile.meta as any).promptInstructions ?? (workflowFile.meta as any).smartQueries ?? (workflowFile.meta as any).smart_queries) + : undefined; + const promptInstructionsForMeta = rawPromptInstructions !== undefined + ? String(rawPromptInstructions).substring(0, 1000) + : undefined; + + const promptLlmProvider = (workflowFile.meta as any).promptLlmProvider; + if (promptLlmProvider && !['anthropic', 'openai', 'ollama'].includes(promptLlmProvider)) { + return res.status(400).json({ error: 'Invalid promptLlmProvider' }); + } @@ - ...((workflowFile.meta as any).promptLlmProvider ? { promptLlmProvider: (workflowFile.meta as any).promptLlmProvider } : {}), + ...(promptLlmProvider ? { promptLlmProvider } : {}), @@ - ...((workflowFile.meta as any).promptLlmApiKey ? { promptLlmApiKey: (workflowFile.meta as any).promptLlmApiKey } : {}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/sdk.ts` around lines 140 - 159, The robot metadata currently pulls promptInstructionsForMeta from workflowFile.meta.smartQueries / smart_queries without normalization; before building robotMeta (and before saving), coerce promptInstructionsForMeta to a safe string (e.g., String(...) or JSON.stringify fallback), trim/cap it to a maximum length, validate/normalize promptLlmProvider against the allowed providers list, and remove or redact promptLlmApiKey/promptLlmBaseUrl from the stored robotMeta (store a boolean like hasPromptLlmApiKey instead) so raw API keys aren’t persisted; make these changes around the promptInstructionsForMeta computation and when spreading into robotMeta (references: promptInstructionsForMeta, robotMeta, workflowFile.meta, promptLlmProvider, promptLlmModel, promptLlmApiKey, promptLlmBaseUrl).server/src/routes/storage.ts (1)
463-526:⚠️ Potential issue | 🟠 MajorValidate and redact LLM config before persisting/capturing it.
This path accepts arbitrary
promptLlmProvider/promptLlmBaseUrland storespromptLlmApiKeyinsiderecording_meta, then sendsrobot_metato analytics. Reject unsupported providers/base URLs up front and do not persist or capture raw API keys.🛡️ Suggested direction
const robotName = (typeof name === 'string' ? name.trim() : '') || `Markdown Robot - ${new URL(url).hostname}`; @@ + if (promptLlmProvider && !['anthropic', 'openai', 'ollama'].includes(promptLlmProvider)) { + return res.status(400).json({ error: 'Invalid promptLlmProvider.' }); + } + const newRobot = await Robot.create({ @@ - ...(promptLlmApiKey ? { promptLlmApiKey } : {}), + // Do not persist raw provider API keys in robot metadata.Also applies to: 538-543
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/routes/storage.ts` around lines 463 - 526, The handler currently accepts arbitrary promptLlmProvider/promptLlmBaseUrl and persists promptLlmApiKey in recording_meta and sends raw robot_meta to analytics; add upfront validation against an allowlist (e.g., ALLOWED_LLM_PROVIDERS and allowed base URL patterns) for promptLlmProvider and promptLlmBaseUrl, reject the request with 400 if they are unsupported, and never persist raw API keys: in Robot.create(recording_meta) remove promptLlmApiKey or replace it with a redacted value (e.g., masked string or boolean flag like hasPromptLlmApiKey:true) and only persist safe fields (promptLlmProvider, promptLlmModel, sanitized promptLlmBaseUrl); additionally ensure any robot_meta sent to analytics is a sanitized copy that excludes or redacts promptLlmApiKey and uses validated promptLlmProvider/promptLlmBaseUrl before calling analytics.src/components/robot/pages/RobotCreate.tsx (1)
477-486:⚠️ Potential issue | 🟠 MajorClear API keys when switching LLM providers.
A key entered for one provider can currently be reused after switching providers, which can submit the wrong secret to the backend and potentially to the wrong LLM API.
🔐 Proposed fix
const provider = e.target.value as 'anthropic' | 'openai' | 'ollama'; setLlmProvider(provider); setLlmModel(''); + setLlmApiKey(''); if (provider === 'ollama') { setLlmBaseUrl('http://localhost:11434'); } else { setLlmBaseUrl(''); }const provider = e.target.value as 'anthropic' | 'openai' | 'ollama'; setScrapePromptLlmProvider(provider); setScrapePromptLlmModel(''); + setScrapePromptLlmApiKey(''); setScrapePromptLlmBaseUrl(provider === 'ollama' ? 'http://localhost:11434' : '');Also applies to: 837-842
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/robot/pages/RobotCreate.tsx` around lines 477 - 486, When changing the LLM provider in the onChange handler, also clear any stored API key state so a key entered for one provider can't be reused for another; update the handler that calls setLlmProvider, setLlmModel, and setLlmBaseUrl to also call the state-clear function(s) (e.g., setLlmApiKey or whatever state holds the provider secret) to reset the key to an empty string, and apply the same change in the other provider-switch handler referenced (the second occurrence around the other onChange block). Ensure you clear all relevant secret fields used by the form so no previous provider secret is submitted.
🧹 Nitpick comments (1)
src/api/storage.ts (1)
31-52: Prefer an options object for SmartQuery settings.Five optional positional parameters after
formatsare easy to misorder. AsmartQuery?: { instructions, provider, model, apiKey, baseUrl }object would make call sites safer and easier to extend.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/storage.ts` around lines 31 - 52, The createScrapeRobot function currently takes five fragile optional positional parameters (promptInstructions, promptLlmProvider, promptLlmModel, promptLlmApiKey, promptLlmBaseUrl) which are easy to misorder; change the signature to accept a single optional smartQuery object (e.g., smartQuery?: { instructions?: string; provider?: 'anthropic'|'openai'|'ollama'; model?: string; apiKey?: string; baseUrl?: string }) and update the axios.post payload to spread smartQuery fields (map instructions -> promptInstructions, provider -> promptLlmProvider, etc.) only when present; update all internal references and call sites of createScrapeRobot to pass a smartQuery object instead of positional prompt* arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/models/Robot.ts`:
- Around line 17-21: The Robot model currently includes a plaintext
promptLlmApiKey which gets saved into recording_meta; remove storing raw API
keys by replacing promptLlmApiKey with a secret reference (e.g.,
promptLlmApiKeyRef or providerSecretId) and/or rely on server-side env/config,
persist only the reference, and ensure serialization/redaction: update the Robot
class/type (symbols: promptLlmApiKey, promptLlmApiKeyRef, recording_meta, Robot)
so the raw key is never written to DB and implement toJSON/serialize logic to
omit/redact any sensitive fields (or map to the secret reference) and add a
migration or validation to strip existing keys from recording_meta before
saving.
In `@server/src/sdk/browserAgent.ts`:
- Around line 593-633: The direct-extraction path is not accounting for token
usage so executeBrowserAgent returns accTokens all zeros when
tryDirectExtraction succeeds; update tryDirectExtraction (or the helper that
invokes callLLMRawText) to also return TokenUsage (or to call the structured
callLLM that returns tokens), then in executeBrowserAgent call addTokens with
the tokens from tryDirectExtraction before returning; specifically modify
tryDirectExtraction/callLLMRawText usage so it provides both the extracted
string and a TokenUsage object and ensure executeBrowserAgent uses
addTokens(acc) to include those tokens in the returned AgentResult.
- Around line 310-318: The code currently uses config.baseUrl (assigned to
baseUrl) directly in axios.post inside browserAgent.ts (around the call site
using AbortController, LLM_TIMEOUT_MS, and axios.post), allowing server-side
requests to arbitrary user-supplied URLs; replace this by validating or
normalizing baseUrl before use: implement and call a helper (e.g.,
isAllowedBaseUrl or normalizeAndValidateBaseUrl) that enforces a server-managed
allowlist or rejects private/link-local/metadata IP ranges and non-HTTPS
schemes, and throw or default to the safe server base URL when validation fails;
apply the same guard wherever config.baseUrl is used (the other axios.post call
sites mentioned) so the backend never issues requests to untrusted endpoints.
- Around line 627-646: The current logs in BrowserAgent expose raw
promptInstructions and full action payloads (see logger.info call that logs
promptInstructions and the one that logs JSON.stringify(action)); update logging
to avoid sensitive data by removing or redacting raw prompt text and action
bodies: log only metadata such as llmConfig.provider, step number, action.type
(from the action object returned by callLLM), and a truncated/sanitized reason
field (e.g., first N chars or run through a redaction helper). Replace direct
logging of promptInstructions and JSON.stringify(action) with calls to a
sanitizer/truncator helper and log tokenUsage or other non-sensitive metadata
instead; ensure buildMessages, tryDirectExtraction, and callLLM usages remain
unchanged.
- Around line 674-685: The code always returns success: true even when the agent
stopped after MAX_STEPS or never produced a finalResult; change the logic to
compute a success boolean and set it to false in those error/fallback cases
(e.g., when step === MAX_STEPS - 1 where you set finalResult to "Agent
stopped..." and when finalResult is set to the "did not return a final result"
fallback). Update the return to return { result: finalResult, steps, success }
instead of always true; use the existing symbols finalResult, MAX_STEPS, step,
and steps to locate and implement this change.
- Around line 268-295: The Anthropic requests in callAnthropic (and other
Anthropic call sites referenced) lack a timeout and can hang; update the
Anthropic client creation in callAnthropic (the Anthropic constructor usage) to
include a timeout option (e.g., timeout: 30000) or pass a per-request timeout
option to anthropic.messages.create, and apply the same change to every other
Anthropic usage (any anthropic.messages.create calls in browserAgent.ts and
workflowEnricher.ts) so each request has a defined timeout to prevent indefinite
waits.
In `@src/components/robot/pages/RobotCreate.tsx`:
- Around line 910-920: The call to createScrapeRobot uses untrimmed values even
though hasPrompt is derived from scrapePromptInstructions.trim(); trim the
payload fields before sending by replacing the raw args in the createScrapeRobot
call with their trimmed equivalents: use scrapePromptInstructions.trim(),
scrapePromptLlmModel.trim() (or undefined if empty), and trim
scrapePromptLlmApiKey and scrapePromptLlmBaseUrl when passing them; keep the
existing hasPrompt checks and only send the trimmed values when hasPrompt is
true to avoid trailing-whitespace failures.
---
Outside diff comments:
In `@server/src/api/record.ts`:
- Around line 803-828: The smartQuery result is written into
serializableOutput.smartQuery by executeBrowserAgent but not exposed in
API/webhook responses; update the response formatter and webhook payload to
include it by adding smartQuery to the object returned by formatRunResponse (and
any similar formatter used for run-start/run-complete payloads) so that
run.update persists and API endpoints (/api/robots/:id/runs) and webhook
builders include serializableOutput.smartQuery in their output; verify any
serialization or typing for formatRunResponse, run-start response builders, and
webhook payload constructors to accept and forward the smartQuery field.
In `@server/src/api/sdk.ts`:
- Around line 140-159: The robot metadata currently pulls
promptInstructionsForMeta from workflowFile.meta.smartQueries / smart_queries
without normalization; before building robotMeta (and before saving), coerce
promptInstructionsForMeta to a safe string (e.g., String(...) or JSON.stringify
fallback), trim/cap it to a maximum length, validate/normalize promptLlmProvider
against the allowed providers list, and remove or redact
promptLlmApiKey/promptLlmBaseUrl from the stored robotMeta (store a boolean like
hasPromptLlmApiKey instead) so raw API keys aren’t persisted; make these changes
around the promptInstructionsForMeta computation and when spreading into
robotMeta (references: promptInstructionsForMeta, robotMeta, workflowFile.meta,
promptLlmProvider, promptLlmModel, promptLlmApiKey, promptLlmBaseUrl).
In `@server/src/pgboss-worker.ts`:
- Around line 312-369: The webhook payload for run completion is missing the
SmartQuery results; update the code that builds webhookPayload (before the
sendWebhook call in the markdown robot completion block) to add
serializableOutput.smartQuery when present (e.g., if
(serializableOutput.smartQuery) webhookPayload.smartQuery =
serializableOutput.smartQuery), ensuring the same SmartQuery value stored
earlier is forwarded in the 'run_completed' webhook sent by sendWebhook.
- Around line 300-327: After executing the long-running SmartQuery
(promptInstructions and executeBrowserAgent), re-check the current run status
before calling run.update(... success ...): reload or re-fetch the run record
(e.g., run.reload() or query by run id) to ensure it wasn't aborted during
SmartQuery, and only proceed to mark status: 'success' and set
finishedAt/log/outputs when the latest run.status is still not 'aborted' (or
other terminal aborted state); if it is aborted, skip the success update and
instead leave the aborted status/log intact or write a resumable message into
serializableOutput.
In `@server/src/routes/storage.ts`:
- Around line 463-526: The handler currently accepts arbitrary
promptLlmProvider/promptLlmBaseUrl and persists promptLlmApiKey in
recording_meta and sends raw robot_meta to analytics; add upfront validation
against an allowlist (e.g., ALLOWED_LLM_PROVIDERS and allowed base URL patterns)
for promptLlmProvider and promptLlmBaseUrl, reject the request with 400 if they
are unsupported, and never persist raw API keys: in Robot.create(recording_meta)
remove promptLlmApiKey or replace it with a redacted value (e.g., masked string
or boolean flag like hasPromptLlmApiKey:true) and only persist safe fields
(promptLlmProvider, promptLlmModel, sanitized promptLlmBaseUrl); additionally
ensure any robot_meta sent to analytics is a sanitized copy that excludes or
redacts promptLlmApiKey and uses validated promptLlmProvider/promptLlmBaseUrl
before calling analytics.
In `@server/src/workflow-management/scheduler/index.ts`:
- Around line 328-400: The webhookPayload currently omits the SmartQuery result
even though serializableOutput.smartQuery is saved; update the code that builds
webhookPayload (the webhookPayload object near where serializableOutput and
uploadedBinaryOutput are available) to add the smart-query data—e.g., if
serializableOutput.smartQuery exists, set webhookPayload.smart_query =
serializableOutput.smartQuery (or webhookPayload.smartQuery if you prefer
camelCase to match other consumers) so scheduled-run webhooks include the
SmartQuery output before the webhook is sent.
In `@src/components/robot/pages/RobotCreate.tsx`:
- Around line 477-486: When changing the LLM provider in the onChange handler,
also clear any stored API key state so a key entered for one provider can't be
reused for another; update the handler that calls setLlmProvider, setLlmModel,
and setLlmBaseUrl to also call the state-clear function(s) (e.g., setLlmApiKey
or whatever state holds the provider secret) to reset the key to an empty
string, and apply the same change in the other provider-switch handler
referenced (the second occurrence around the other onChange block). Ensure you
clear all relevant secret fields used by the form so no previous provider secret
is submitted.
---
Nitpick comments:
In `@src/api/storage.ts`:
- Around line 31-52: The createScrapeRobot function currently takes five fragile
optional positional parameters (promptInstructions, promptLlmProvider,
promptLlmModel, promptLlmApiKey, promptLlmBaseUrl) which are easy to misorder;
change the signature to accept a single optional smartQuery object (e.g.,
smartQuery?: { instructions?: string; provider?: 'anthropic'|'openai'|'ollama';
model?: string; apiKey?: string; baseUrl?: string }) and update the axios.post
payload to spread smartQuery fields (map instructions -> promptInstructions,
provider -> promptLlmProvider, etc.) only when present; update all internal
references and call sites of createScrapeRobot to pass a smartQuery object
instead of positional prompt* arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bdbe57a7-ebd8-4180-bf9c-15399c2eab6a
📒 Files selected for processing (9)
server/src/api/record.tsserver/src/api/sdk.tsserver/src/models/Robot.tsserver/src/pgboss-worker.tsserver/src/routes/storage.tsserver/src/sdk/browserAgent.tsserver/src/workflow-management/scheduler/index.tssrc/api/storage.tssrc/components/robot/pages/RobotCreate.tsx
Comment on lines
+17
to
+21
| promptInstructions?: string; | ||
| promptLlmProvider?: 'anthropic' | 'openai' | 'ollama'; | ||
| promptLlmModel?: string; | ||
| promptLlmApiKey?: string; | ||
| promptLlmBaseUrl?: string; |
There was a problem hiding this comment.
Avoid storing provider API keys in robot metadata.
promptLlmApiKey becomes part of recording_meta, which is persisted as JSONB and commonly returned/captured with robot metadata. Store an encrypted secret reference or rely on server-side environment/config instead, and redact this field from any serialized robot metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/models/Robot.ts` around lines 17 - 21, The Robot model currently
includes a plaintext promptLlmApiKey which gets saved into recording_meta;
remove storing raw API keys by replacing promptLlmApiKey with a secret reference
(e.g., promptLlmApiKeyRef or providerSecretId) and/or rely on server-side
env/config, persist only the reference, and ensure serialization/redaction:
update the Robot class/type (symbols: promptLlmApiKey, promptLlmApiKeyRef,
recording_meta, Robot) so the raw key is never written to DB and implement
toJSON/serialize logic to omit/redact any sensitive fields (or map to the secret
reference) and add a migration or validation to strip existing keys from
recording_meta before saving.
Comment on lines
+268
to
+295
| async function callAnthropic(config: LLMConfig, messages: any[]): Promise<LLMCallResult> { | ||
| const anthropic = new Anthropic({ apiKey: config.apiKey || process.env.ANTHROPIC_API_KEY }); | ||
| const model = config.model || 'claude-3-5-sonnet-20241022'; | ||
|
|
||
| const systemMsg = messages.find((m: any) => m.role === 'system'); | ||
| const userMsg = messages.find((m: any) => m.role === 'user'); | ||
|
|
||
| const anthropicContent: any[] = []; | ||
| if (Array.isArray(userMsg.content)) { | ||
| for (const block of userMsg.content) { | ||
| if (block.type === 'image_url') { | ||
| const raw: string = block.image_url?.url || ''; | ||
| const base64 = raw.includes(',') ? raw.split(',')[1] : raw; | ||
| anthropicContent.push({ type: 'image', source: { type: 'base64', media_type: 'image/jpeg', data: base64 } }); | ||
| } else if (block.type === 'text') { | ||
| anthropicContent.push({ type: 'text', text: block.text }); | ||
| } | ||
| } | ||
| } else { | ||
| anthropicContent.push({ type: 'text', text: userMsg.content }); | ||
| } | ||
|
|
||
| const response = await anthropic.messages.create({ | ||
| model, | ||
| max_tokens: 256, | ||
| system: systemMsg?.content || SYSTEM_PROMPT, | ||
| messages: [{ role: 'user', content: anthropicContent }], | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect Anthropic SDK version and call sites.
rg -n '"@anthropic-ai/sdk"|anthropic\.messages\.create|new Anthropic' -C 3Repository: getmaxun/maxun
Length of output: 7815
🏁 Script executed:
#!/bin/bash
# Check if timeouts are already implemented in browserAgent.ts
cat -n server/src/sdk/browserAgent.ts | sed -n '265,300p'Repository: getmaxun/maxun
Length of output: 1695
🏁 Script executed:
#!/bin/bash
# Check the second location mentioned (lines 412-425)
cat -n server/src/sdk/browserAgent.ts | sed -n '410,430p'Repository: getmaxun/maxun
Length of output: 1062
🏁 Script executed:
#!/bin/bash
# Check OpenAI call sites to see timeout pattern
rg -n 'openai\.chat\.completions\.create|new OpenAI|timeout' server/src/sdk/browserAgent.ts -A 5 -B 2Repository: getmaxun/maxun
Length of output: 2418
🌐 Web query:
Anthropic SDK v0.71.2 timeout configuration options
💡 Result:
The Anthropic Python SDK (anthropic-sdk-python) v0.71.2 supports timeout configuration through the timeout parameter in the Anthropic client constructor and per-request options. This has been consistent across versions, including around v0.71.x (released circa late 2025), as evidenced by source code at relevant commits and official documentation. ### Client-Level Configuration (default applies to all requests) python from anthropic import Anthropic import httpx # Simple float (total timeout in seconds, default ~600s / 10 minutes) client = Anthropic( api_key="your_api_key", timeout=20.0, # 20 seconds total ) # Granular httpx.Timeout client = Anthropic( api_key="your_api_key", timeout=httpx.Timeout( connect=2.0, # Connection timeout read=5.0, # Read timeout write=10.0, # Write timeout total=60.0 # Overall timeout ), ) ### Per-Request Override Use with_options or pass timeout directly to methods like messages.create: python # Using with_options client.with_options(timeout=5.0).messages.create( model="claude-3-5-sonnet-20240620", max_tokens=1024, messages=[{"role": "user", "content": "Hello"}] ) # Direct in method (overrides client default) client.messages.create( ..., timeout=httpx.Timeout(10.0, read=3.0), ) ### Additional Notes - Default: 10 minutes total. For non-streaming requests expected >10min (based on max_tokens), SDK may raise ValueError unless stream=True or timeout overridden. - Retries: Timeouts trigger retries (default max_retries=2). On timeout, raises APITimeoutError. - Streaming: Recommended for long requests to avoid idle connection drops. - No version-specific changes to timeout in v0.71.x per CHANGELOG; uses httpx under the hood. This applies to both sync Anthropic and async AsyncAnthropic clients.
Citations:
- 1: https://platform.claude.com/docs/en/api/sdks/python
- 2: https://github.com/anthropics/anthropic-sdk-python/blob/8e3d8a68d309424238ae54e03ee962f7147cfc60/src/anthropic/_client.py
- 3: https://github.com/anthropics/anthropic-sdk-python/blob/49d639a6/src/anthropic/_client.py
- 4: https://github.com/anthropics/anthropic-sdk-python/blob/main/src/anthropic/_client.py
- 5: https://github.com/anthropics/anthropic-sdk-python/blob/main/src/anthropic/resources/beta/messages/messages.py
- 6: https://github.com/anthropics/anthropic-sdk-python/blob/main/CHANGELOG.md
🌐 Web query:
Anthropic JavaScript SDK @anthropic-ai/sdk v0.71.2 timeout configuration messages.create
💡 Result:
In @anthropic-ai/sdk v0.71.2, timeout configuration for client.messages.create is set via the ClientOptions timeout parameter (in milliseconds, default 600000 / 10 minutes) when instantiating the Anthropic client, or overridden per-request via RequestOptions. Client-level: import Anthropic from '@anthropic-ai/sdk'; const client = new Anthropic({ timeout: 120000, // 2 minutes }); const message = await client.messages.create({ model: 'claude-3-5-sonnet-20241022', max_tokens: 1024, messages: [{ role: 'user', content: 'Hello!' }] }); Per-request: const message = await client.messages.create( { /* params */ }, { timeout: 30000 } // 30 seconds, overrides client default ); For non-streaming requests with large max_tokens, timeout is dynamically calculated as min(10 minutes, (3600 * max_tokens) / 128000 seconds), unless overridden. Use streaming (stream: true) for long-running requests to avoid timeouts. On timeout, APIConnectionTimeoutError is thrown (retried by default up to maxRetries=2). Also available via client.withOptions({ timeout: ... }).messages.create(...). v0.71.2 released 2025-12-05; timeout behavior consistent across recent versions per changelog.
Citations:
- 1: https://tessl.io/registry/tessl/npm-anthropic-ai--sdk/0.70.0/files/docs/client.md
- 2: https://tessl.io/registry/tessl/npm-anthropic-ai--sdk/0.61.0/docs/client-configuration.md
- 3: https://github.com/anthropics/anthropic-sdk-typescript/blob/e400d2e8a54aa736717ed849ef8b44a3490fce68/src/index.ts
- 4: https://github.com/anthropics/anthropic-sdk-typescript/blob/main/src/client.ts
- 5: https://console.anthropic.com/docs/en/api/sdks/typescript
- 6: https://platform.claude.com/docs/en/api/sdks/typescript
- 7: https://github.com/anthropics/anthropic-sdk-typescript/blob/main/CHANGELOG.md
🏁 Script executed:
#!/bin/bash
# Search for OpenAI calls and their timeout implementations
rg -n 'openai\.chat\.completions\.create|new OpenAI' server/src/sdk/browserAgent.ts -A 8Repository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check if any Anthropic calls already have timeout
rg -n 'anthropic\.messages\.create' server/src/sdk/browserAgent.ts -A 3Repository: getmaxun/maxun
Length of output: 359
Add timeout configuration to Anthropic SDK calls to prevent indefinite hangs.
Anthropic API requests lack timeout protection, which can cause the worker to hang indefinitely under network or provider issues. The Anthropic JavaScript SDK (v0.71.2) supports timeout configuration both at client instantiation and per-request.
Configure timeout either when creating the client:
const anthropic = new Anthropic({
apiKey: config.apiKey || process.env.ANTHROPIC_API_KEY,
timeout: 30000 // 30 seconds
});Or override per-request:
const response = await anthropic.messages.create(
{ /* existing params */ },
{ timeout: 30000 }
);This applies to all Anthropic call sites: lines 268–295 (browserAgent.ts), 412–425 (browserAgent.ts), and 6 additional locations in workflowEnricher.ts (lines 502–510, 841–849, 1053–1061, 1328–1336, 1700–1708, 1971–1979).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/sdk/browserAgent.ts` around lines 268 - 295, The Anthropic
requests in callAnthropic (and other Anthropic call sites referenced) lack a
timeout and can hang; update the Anthropic client creation in callAnthropic (the
Anthropic constructor usage) to include a timeout option (e.g., timeout: 30000)
or pass a per-request timeout option to anthropic.messages.create, and apply the
same change to every other Anthropic usage (any anthropic.messages.create calls
in browserAgent.ts and workflowEnricher.ts) so each request has a defined
timeout to prevent indefinite waits.
Comment on lines
+310
to
+318
| const baseUrl = config.baseUrl || 'https://api.openai.com/v1'; | ||
| const model = config.model || 'gpt-4o'; | ||
|
|
||
| const controller = new AbortController(); | ||
| const tid = setTimeout(() => controller.abort(), LLM_TIMEOUT_MS); | ||
|
|
||
| try { | ||
| const response = await axios.post( | ||
| `${baseUrl}/chat/completions`, |
There was a problem hiding this comment.
Do not call arbitrary user-provided LLM base URLs.
config.baseUrl is passed directly into server-side axios.post(...), letting an authenticated user make the backend POST to internal services. Restrict this to an allowlist/server-managed endpoints, or reject private/link-local/metadata IP ranges before using it.
🛡️ Suggested guard
+function resolveAllowedBaseUrl(rawBaseUrl: string | undefined, fallback: string): string {
+ const baseUrl = rawBaseUrl || fallback;
+ const parsed = new URL(baseUrl);
+ const allowedOrigins = (process.env.ALLOWED_LLM_BASE_URLS || '')
+ .split(',')
+ .map(v => v.trim())
+ .filter(Boolean);
+
+ if (allowedOrigins.length > 0 && !allowedOrigins.includes(parsed.origin)) {
+ throw new Error(`LLM base URL is not allowed: ${parsed.origin}`);
+ }
+
+ return parsed.origin;
+}Also applies to: 341-368, 430-467
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/sdk/browserAgent.ts` around lines 310 - 318, The code currently
uses config.baseUrl (assigned to baseUrl) directly in axios.post inside
browserAgent.ts (around the call site using AbortController, LLM_TIMEOUT_MS, and
axios.post), allowing server-side requests to arbitrary user-supplied URLs;
replace this by validating or normalizing baseUrl before use: implement and call
a helper (e.g., isAllowedBaseUrl or normalizeAndValidateBaseUrl) that enforces a
server-managed allowlist or rejects private/link-local/metadata IP ranges and
non-HTTPS schemes, and throw or default to the safe server base URL when
validation fails; apply the same guard wherever config.baseUrl is used (the
other axios.post call sites mentioned) so the backend never issues requests to
untrusted endpoints.
Comment on lines
+593
to
+633
| const raw = await callLLMRawText(messages, config); | ||
| if (!raw) return null; | ||
| const trimmed = raw.trim(); | ||
| if (trimmed.toUpperCase().startsWith('ANSWER:')) { | ||
| const answer = trimmed.substring(7).trim(); | ||
| if (answer.length > 0) { | ||
| logger.info(`[BrowserAgent] Direct extraction succeeded: "${answer.substring(0, 200)}"`); | ||
| return answer; | ||
| } | ||
| } | ||
| logger.info('[BrowserAgent] Model indicated page interaction is needed'); | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * Execute prompt instructions on an open browser page using an LLM agent. | ||
| * First attempts direct extraction from page text; falls back to the full | ||
| * observe → decide → act loop only when page interaction is genuinely needed. | ||
| */ | ||
| export async function executeBrowserAgent( | ||
| page: Page, | ||
| promptInstructions: string, | ||
| llmConfig: LLMConfig | ||
| ): Promise<AgentResult> { | ||
| const steps: AgentStep[] = []; | ||
| let finalResult = ''; | ||
| const accTokens: TokenUsage = { promptTokens: 0, completionTokens: 0, totalTokens: 0 }; | ||
|
|
||
| function addTokens(t: TokenUsage) { | ||
| accTokens.promptTokens += t.promptTokens; | ||
| accTokens.completionTokens += t.completionTokens; | ||
| accTokens.totalTokens += t.totalTokens; | ||
| } | ||
|
|
||
| logger.info(`[BrowserAgent] Starting agent. Provider: ${llmConfig.provider}. Prompt: "${promptInstructions.substring(0, 120)}"`); | ||
|
|
||
| const initialPageState = await capturePageState(page); | ||
| const directResult = await tryDirectExtraction(initialPageState, promptInstructions, llmConfig); | ||
| if (directResult !== null) { | ||
| logger.info(`[BrowserAgent] Token usage — prompt: ${accTokens.promptTokens}, completion: ${accTokens.completionTokens}, total: ${accTokens.totalTokens}`); | ||
| return { result: directResult, steps: [], success: true, tokenUsage: accTokens }; |
There was a problem hiding this comment.
Account for tokens in the direct-extraction path.
tryDirectExtraction(...) calls the LLM but only returns text, so direct successes report tokenUsage as all zeros. Return tokens from callLLMRawText or use the structured callLLM path for this probe.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/sdk/browserAgent.ts` around lines 593 - 633, The direct-extraction
path is not accounting for token usage so executeBrowserAgent returns accTokens
all zeros when tryDirectExtraction succeeds; update tryDirectExtraction (or the
helper that invokes callLLMRawText) to also return TokenUsage (or to call the
structured callLLM that returns tokens), then in executeBrowserAgent call
addTokens with the tokens from tryDirectExtraction before returning;
specifically modify tryDirectExtraction/callLLMRawText usage so it provides both
the extracted string and a TokenUsage object and ensure executeBrowserAgent uses
addTokens(acc) to include those tokens in the returned AgentResult.
Comment on lines
+627
to
+646
| logger.info(`[BrowserAgent] Starting agent. Provider: ${llmConfig.provider}. Prompt: "${promptInstructions.substring(0, 120)}"`); | ||
|
|
||
| const initialPageState = await capturePageState(page); | ||
| const directResult = await tryDirectExtraction(initialPageState, promptInstructions, llmConfig); | ||
| if (directResult !== null) { | ||
| logger.info(`[BrowserAgent] Token usage — prompt: ${accTokens.promptTokens}, completion: ${accTokens.completionTokens}, total: ${accTokens.totalTokens}`); | ||
| return { result: directResult, steps: [], success: true, tokenUsage: accTokens }; | ||
| } | ||
|
|
||
| logger.info('[BrowserAgent] Direct extraction insufficient — starting action loop'); | ||
|
|
||
| for (let step = 0; step < MAX_STEPS; step++) { | ||
| try { | ||
| const pageState = step === 0 ? initialPageState : await capturePageState(page); | ||
| const messages = buildMessages(promptInstructions, pageState, steps); | ||
|
|
||
| const { action, tokens } = await callLLM(messages, llmConfig); | ||
| addTokens(tokens); | ||
|
|
||
| logger.info(`[BrowserAgent] Step ${step + 1}: ${JSON.stringify(action)}`); |
There was a problem hiding this comment.
Avoid logging raw prompts and action payloads.
promptInstructions and JSON.stringify(action) may contain user-provided secrets, search terms, or typed form values. Log only metadata such as provider, step number, action type, and truncated non-sensitive reason.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/sdk/browserAgent.ts` around lines 627 - 646, The current logs in
BrowserAgent expose raw promptInstructions and full action payloads (see
logger.info call that logs promptInstructions and the one that logs
JSON.stringify(action)); update logging to avoid sensitive data by removing or
redacting raw prompt text and action bodies: log only metadata such as
llmConfig.provider, step number, action.type (from the action object returned by
callLLM), and a truncated/sanitized reason field (e.g., first N chars or run
through a redaction helper). Replace direct logging of promptInstructions and
JSON.stringify(action) with calls to a sanitizer/truncator helper and log
tokenUsage or other non-sensitive metadata instead; ensure buildMessages,
tryDirectExtraction, and callLLM usages remain unchanged.
Comment on lines
+674
to
+685
| if (step === MAX_STEPS - 1) { | ||
| finalResult = `Agent stopped after ${step + 1} steps. Last error: ${shortError}`; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (!finalResult) { | ||
| finalResult = `Agent completed ${steps.length} step(s) but did not return a final result.`; | ||
| } | ||
|
|
||
| logger.info(`[BrowserAgent] Token usage — prompt: ${accTokens.promptTokens}, completion: ${accTokens.completionTokens}, total: ${accTokens.totalTokens}`); | ||
| return { result: finalResult, steps, success: true, tokenUsage: accTokens }; |
There was a problem hiding this comment.
Return success: false when the agent never completes.
The fallback/error paths still return success: true, so callers cannot distinguish a real answer from “stopped after errors” or “did not return a final result.”
🐛 Suggested fix
const steps: AgentStep[] = [];
let finalResult = '';
+ let success = false;
@@
if (action.action === 'done') {
finalResult = action.result || '';
+ success = true;
logger.info(`[BrowserAgent] Done in ${step + 1} step(s). Result: "${finalResult.substring(0, 200)}"`);
break;
}
@@
if (step === MAX_STEPS - 1) {
finalResult = `Agent stopped after ${step + 1} steps. Last error: ${shortError}`;
+ success = false;
}
@@
if (!finalResult) {
finalResult = `Agent completed ${steps.length} step(s) but did not return a final result.`;
+ success = false;
}
@@
- return { result: finalResult, steps, success: true, tokenUsage: accTokens };
+ return { result: finalResult, steps, success, tokenUsage: accTokens };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/sdk/browserAgent.ts` around lines 674 - 685, The code always
returns success: true even when the agent stopped after MAX_STEPS or never
produced a finalResult; change the logic to compute a success boolean and set it
to false in those error/fallback cases (e.g., when step === MAX_STEPS - 1 where
you set finalResult to "Agent stopped..." and when finalResult is set to the
"did not return a final result" fallback). Update the return to return { result:
finalResult, steps, success } instead of always true; use the existing symbols
finalResult, MAX_STEPS, step, and steps to locate and implement this change.
Comment on lines
+910
to
+920
| const hasPrompt = !!scrapePromptInstructions.trim(); | ||
| const result = await createScrapeRobot( | ||
| url, | ||
| scrapeRobotName, | ||
| outputFormats, | ||
| hasPrompt ? scrapePromptInstructions : undefined, | ||
| hasPrompt ? scrapePromptLlmProvider : undefined, | ||
| hasPrompt ? scrapePromptLlmModel.trim() || undefined : undefined, | ||
| hasPrompt && scrapePromptLlmProvider !== 'ollama' ? scrapePromptLlmApiKey || undefined : undefined, | ||
| hasPrompt && scrapePromptLlmProvider === 'ollama' ? scrapePromptLlmBaseUrl || undefined : undefined, | ||
| ); |
There was a problem hiding this comment.
Trim Smart Query payload fields before sending.
hasPrompt is based on the trimmed prompt, but the request sends the raw prompt/API key/base URL. Trailing whitespace from copy-paste can cause avoidable Smart Query failures.
🧹 Proposed fix
- const hasPrompt = !!scrapePromptInstructions.trim();
+ const promptInstructions = scrapePromptInstructions.trim();
+ const promptLlmApiKey = scrapePromptLlmApiKey.trim();
+ const promptLlmBaseUrl = scrapePromptLlmBaseUrl.trim();
+ const hasPrompt = !!promptInstructions;
const result = await createScrapeRobot(
url,
scrapeRobotName,
outputFormats,
- hasPrompt ? scrapePromptInstructions : undefined,
+ hasPrompt ? promptInstructions : undefined,
hasPrompt ? scrapePromptLlmProvider : undefined,
hasPrompt ? scrapePromptLlmModel.trim() || undefined : undefined,
- hasPrompt && scrapePromptLlmProvider !== 'ollama' ? scrapePromptLlmApiKey || undefined : undefined,
- hasPrompt && scrapePromptLlmProvider === 'ollama' ? scrapePromptLlmBaseUrl || undefined : undefined,
+ hasPrompt && scrapePromptLlmProvider !== 'ollama' ? promptLlmApiKey || undefined : undefined,
+ hasPrompt && scrapePromptLlmProvider === 'ollama' ? promptLlmBaseUrl || undefined : undefined,
);📝 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.
Suggested change
| const hasPrompt = !!scrapePromptInstructions.trim(); | |
| const result = await createScrapeRobot( | |
| url, | |
| scrapeRobotName, | |
| outputFormats, | |
| hasPrompt ? scrapePromptInstructions : undefined, | |
| hasPrompt ? scrapePromptLlmProvider : undefined, | |
| hasPrompt ? scrapePromptLlmModel.trim() || undefined : undefined, | |
| hasPrompt && scrapePromptLlmProvider !== 'ollama' ? scrapePromptLlmApiKey || undefined : undefined, | |
| hasPrompt && scrapePromptLlmProvider === 'ollama' ? scrapePromptLlmBaseUrl || undefined : undefined, | |
| ); | |
| const promptInstructions = scrapePromptInstructions.trim(); | |
| const promptLlmApiKey = scrapePromptLlmApiKey.trim(); | |
| const promptLlmBaseUrl = scrapePromptLlmBaseUrl.trim(); | |
| const hasPrompt = !!promptInstructions; | |
| const result = await createScrapeRobot( | |
| url, | |
| scrapeRobotName, | |
| outputFormats, | |
| hasPrompt ? promptInstructions : undefined, | |
| hasPrompt ? scrapePromptLlmProvider : undefined, | |
| hasPrompt ? scrapePromptLlmModel.trim() || undefined : undefined, | |
| hasPrompt && scrapePromptLlmProvider !== 'ollama' ? promptLlmApiKey || undefined : undefined, | |
| hasPrompt && scrapePromptLlmProvider === 'ollama' ? promptLlmBaseUrl || undefined : undefined, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/robot/pages/RobotCreate.tsx` around lines 910 - 920, The call
to createScrapeRobot uses untrimmed values even though hasPrompt is derived from
scrapePromptInstructions.trim(); trim the payload fields before sending by
replacing the raw args in the createScrapeRobot call with their trimmed
equivalents: use scrapePromptInstructions.trim(), scrapePromptLlmModel.trim()
(or undefined if empty), and trim scrapePromptLlmApiKey and
scrapePromptLlmBaseUrl when passing them; keep the existing hasPrompt checks and
only send the trimmed values when hasPrompt is true to avoid trailing-whitespace
failures.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
server/src/sdk/browserAgent.ts (5)
593-633:⚠️ Potential issue | 🟡 MinorAccount for tokens on direct-extraction success.
tryDirectExtractionstill returns only the answer text, so successful direct extraction returns zero token usage even though an LLM call was made. Return{ answer, tokens }from the raw path and calladdTokens(...)before returning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/sdk/browserAgent.ts` around lines 593 - 633, tryDirectExtraction currently returns only the answer text causing accTokens to remain zero; change tryDirectExtraction to return both the extracted answer and the TokenUsage (e.g., { answer, tokens }) from the raw/LLM path (identify callLLMRawText and the raw/trimmed/ANSWER: branch inside tryDirectExtraction), then in executeBrowserAgent use the new shape: call tryDirectExtraction, if it returns a non-null object call addTokens(returned.tokens) before logging/returning and populate AgentResult.result with returned.answer; ensure function signatures and any callers are updated to accept the { answer, tokens } return shape.
674-685:⚠️ Potential issue | 🟠 MajorReturn failure when the agent stops without a final answer.
The fallback paths still return
success: true, so callers cannot distinguish a completed answer from max-step exhaustion or nodoneresult.🐛 Suggested success tracking
const steps: AgentStep[] = []; let finalResult = ''; + let success = false; const accTokens: TokenUsage = { promptTokens: 0, completionTokens: 0, totalTokens: 0 }; @@ if (action.action === 'done') { finalResult = action.result || ''; + success = finalResult.length > 0; logger.info(`[BrowserAgent] Done in ${step + 1} step(s). Result: "${finalResult.substring(0, 200)}"`); break; } @@ if (step === MAX_STEPS - 1) { finalResult = `Agent stopped after ${step + 1} steps. Last error: ${shortError}`; + success = false; } @@ if (!finalResult) { finalResult = `Agent completed ${steps.length} step(s) but did not return a final result.`; + success = false; } logger.info(`[BrowserAgent] Token usage — prompt: ${accTokens.promptTokens}, completion: ${accTokens.completionTokens}, total: ${accTokens.totalTokens}`); - return { result: finalResult, steps, success: true, tokenUsage: accTokens }; + return { result: finalResult, steps, success, tokenUsage: accTokens }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/sdk/browserAgent.ts` around lines 674 - 685, The function currently always returns success: true even when the agent exhausted steps or produced no finalResult; update the return logic to set success = false in failure cases: when finalResult is set due to MAX_STEPS exhaustion (the branch that assigns finalResult = `Agent stopped after ${step + 1} steps...`) and when the fallback assigns finalResult = `Agent completed ${steps.length} step(s) but did not return a final result.`; locate and modify the variables finalResult and the return object (result, steps, success, tokenUsage) so that success is false for these two conditions (and keep success true only when a genuine done/final result was produced), and ensure the logger.info call remains unchanged.
599-599:⚠️ Potential issue | 🟠 MajorAvoid logging raw prompts, actions, and extracted results.
These values can contain scraped PII, user search terms, or typed form data. Log metadata only: provider, step number, action type, result length, and sanitized/truncated non-sensitive reason text.
🔒 Suggested logging direction
- logger.info(`[BrowserAgent] Direct extraction succeeded: "${answer.substring(0, 200)}"`); + logger.info(`[BrowserAgent] Direct extraction succeeded. Result length: ${answer.length}`);- logger.info(`[BrowserAgent] Starting agent. Provider: ${llmConfig.provider}. Prompt: "${promptInstructions.substring(0, 120)}"`); + logger.info(`[BrowserAgent] Starting agent. Provider: ${llmConfig.provider}. Prompt length: ${promptInstructions.length}`);- logger.info(`[BrowserAgent] Step ${step + 1}: ${JSON.stringify(action)}`); + logger.info(`[BrowserAgent] Step ${step + 1}: action=${action.action}`);- logger.info(`[BrowserAgent] Done in ${step + 1} step(s). Result: "${finalResult.substring(0, 200)}"`); + logger.info(`[BrowserAgent] Done in ${step + 1} step(s). Result length: ${finalResult.length}`);Also applies to: 627-646, 658-658
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/sdk/browserAgent.ts` at line 599, The logger.info call that prints the raw extracted text (logger.info(`[BrowserAgent] Direct extraction succeeded: "${answer.substring(0, 200)}"`)) and similar logs in the same file must be replaced with metadata-only logs: do not include raw prompts, actions, or extracted results. Update the logging in BrowserAgent (the logger.info at the shown extraction success and the related blocks around lines 627-646 and 658) to log only provider, step number, action type, result length (e.g., answer.length), and a sanitized/truncated non-sensitive reason string (use or add a sanitizeLog/sanitizeForLogging helper that strips PII and truncates to N chars), and remove any direct interpolation of answer, prompt, or action payloads from log messages. Ensure consistency by using the same helper for all affected log sites in this file.
268-295:⚠️ Potential issue | 🟠 MajorFinish applying request timeouts to every LLM path.
The Anthropic calls and the raw OpenAI direct-extraction call can still wait indefinitely;
callOpenAIhas timeout protection, butcallLLMRawTextdoes not. Apply the same timeout policy consistently.#!/bin/bash # Verify LLM call sites and timeout coverage in this file. rg -n 'new Anthropic|anthropic\.messages\.create|axios\.post|AbortController|LLM_TIMEOUT_MS' -C 3 server/src/sdk/browserAgent.ts⏱️ Suggested timeout fix
- const anthropic = new Anthropic({ apiKey: config.apiKey || process.env.ANTHROPIC_API_KEY }); + const anthropic = new Anthropic({ + apiKey: config.apiKey || process.env.ANTHROPIC_API_KEY, + timeout: LLM_TIMEOUT_MS, + });- const response = await axios.post( - `${baseUrl}/chat/completions`, - { model, messages: textMessages, max_tokens: 512, temperature: 0.1 }, - { - headers: { - 'Authorization': `Bearer ${config.apiKey || process.env.OPENAI_API_KEY}`, - 'Content-Type': 'application/json', - }, - } - ); + const controller = new AbortController(); + const tid = setTimeout(() => controller.abort(), LLM_TIMEOUT_MS); + try { + const response = await axios.post( + `${baseUrl}/chat/completions`, + { model, messages: textMessages, max_tokens: 512, temperature: 0.1 }, + { + headers: { + 'Authorization': `Bearer ${config.apiKey || process.env.OPENAI_API_KEY}`, + 'Content-Type': 'application/json', + }, + signal: controller.signal as any, + } + ); + return response.data.choices?.[0]?.message?.content || null; + } finally { + clearTimeout(tid); + }Also applies to: 412-425, 430-450
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/sdk/browserAgent.ts` around lines 268 - 295, The Anthropic call (callAnthropic -> anthropic.messages.create) and other raw LLM paths (callLLMRawText, any direct axios.post LLM requests) lack request timeouts; wrap each LLM request in the same timeout policy using the existing LLM_TIMEOUT_MS and an AbortController (or Promise.race timeout if the SDK call doesn't accept a signal): create an AbortController, set a timer to call controller.abort() after LLM_TIMEOUT_MS, pass controller.signal to anthropic.messages.create or to axios (signal option) or reject the race on timeout, clear the timer on success, and surface a consistent timeout error; apply this pattern to callAnthropic, callLLMRawText, and any axios.post usages referenced (anthropic.messages.create / new Anthropic / axios.post).
309-318:⚠️ Potential issue | 🔴 CriticalDo not POST to arbitrary user-configured LLM base URLs.
promptLlmBaseUrlflows intoconfig.baseUrl, then into server-sideaxios.post(...). Keep this behind a server-managed allowlist and reject private/link-local/metadata ranges before making outbound requests.Also applies to: 341-368, 430-467
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/sdk/browserAgent.ts` around lines 309 - 318, The code currently allows arbitrary config.baseUrl to flow into server-side axios.post in callOpenAI (and similar call sites), so enforce a server-managed allowlist and network-block checks before making outbound requests: ensure config.baseUrl is only accepted if it matches an explicit allowlist of approved origins (use a centralized ALLOWLIST and default to the safe OpenAI URL when absent), parse the baseUrl with URL to extract hostname, resolve the hostname to IP(s) and reject any addresses in private, link-local, loopback, or metadata ranges (e.g., 127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16, 169.254.169.254, ::1, fc00::/7, fe80::/10), and also block hostnames that resolve to such IPs; if validation fails, throw a clear error and do not call axios.post; apply this validation in callOpenAI and the other LLM call functions (the same pattern used around config.baseUrl) so only approved, non-private endpoints are contacted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/sdk/browserAgent.ts`:
- Around line 88-103: extractJson currently treats malformed or non-JSON LLM
output as a successful "done" and executeAction allows unknown/unsafe actions;
change this to validate and reject untrusted model output before executing: in
extractJson and its callers (e.g., executeAction) ensure parsed JSON matches an
explicit schema (allowed action names like "click","type","navigate","done"),
require the selector field to match /^\[data-agent-idx="(\d+)"\]$/ and
coerce/serialize non-string result fields rather than silently accepting them,
and for "navigate" actions restrict targets to an allowlist of safe origins or
relative paths; if validation fails, return a standardized error action (not
"done") and do not proceed to execution. Use the function names extractJson and
executeAction to locate the logic to update.
- Around line 513-516: The navigate handler (case 'navigate' in
server/src/sdk/browserAgent.ts) currently allows arbitrary URLs; restrict it by
validating action.url before calling page.goto: parse the URL with new
URL(action.url, currentPageUrl), require protocol === 'http:' || 'https:', then
either enforce same-origin (compare parsedUrl.origin to currentPageUrl.origin)
or check parsedUrl.origin/hostname against a server-managed allowlist function
(e.g., isAllowedOrigin/allowlist). Also reject hostnames or resolved IPs in
private, loopback, link-local or cloud metadata ranges (e.g., 127.0.0.0/8,
10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16, fe80::/10, and
169.254.169.254), using DNS resolution/net.isIP to detect numeric addresses and
blacklist ranges, and throw an error if validation fails instead of calling
page.goto.
- Around line 536-539: In the 'check' case inside the switch handling browser
actions (the block that currently throws "check: missing selector" and calls
page.click), replace the use of page.click(action.selector, { timeout: 5000 })
with page.check(action.selector, { timeout: 5000 }) so the operation is
idempotent and only checks the element if not already checked; keep the existing
selector validation and timeout behavior around the page.check call.
---
Duplicate comments:
In `@server/src/sdk/browserAgent.ts`:
- Around line 593-633: tryDirectExtraction currently returns only the answer
text causing accTokens to remain zero; change tryDirectExtraction to return both
the extracted answer and the TokenUsage (e.g., { answer, tokens }) from the
raw/LLM path (identify callLLMRawText and the raw/trimmed/ANSWER: branch inside
tryDirectExtraction), then in executeBrowserAgent use the new shape: call
tryDirectExtraction, if it returns a non-null object call
addTokens(returned.tokens) before logging/returning and populate
AgentResult.result with returned.answer; ensure function signatures and any
callers are updated to accept the { answer, tokens } return shape.
- Around line 674-685: The function currently always returns success: true even
when the agent exhausted steps or produced no finalResult; update the return
logic to set success = false in failure cases: when finalResult is set due to
MAX_STEPS exhaustion (the branch that assigns finalResult = `Agent stopped after
${step + 1} steps...`) and when the fallback assigns finalResult = `Agent
completed ${steps.length} step(s) but did not return a final result.`; locate
and modify the variables finalResult and the return object (result, steps,
success, tokenUsage) so that success is false for these two conditions (and keep
success true only when a genuine done/final result was produced), and ensure the
logger.info call remains unchanged.
- Line 599: The logger.info call that prints the raw extracted text
(logger.info(`[BrowserAgent] Direct extraction succeeded: "${answer.substring(0,
200)}"`)) and similar logs in the same file must be replaced with metadata-only
logs: do not include raw prompts, actions, or extracted results. Update the
logging in BrowserAgent (the logger.info at the shown extraction success and the
related blocks around lines 627-646 and 658) to log only provider, step number,
action type, result length (e.g., answer.length), and a sanitized/truncated
non-sensitive reason string (use or add a sanitizeLog/sanitizeForLogging helper
that strips PII and truncates to N chars), and remove any direct interpolation
of answer, prompt, or action payloads from log messages. Ensure consistency by
using the same helper for all affected log sites in this file.
- Around line 268-295: The Anthropic call (callAnthropic ->
anthropic.messages.create) and other raw LLM paths (callLLMRawText, any direct
axios.post LLM requests) lack request timeouts; wrap each LLM request in the
same timeout policy using the existing LLM_TIMEOUT_MS and an AbortController (or
Promise.race timeout if the SDK call doesn't accept a signal): create an
AbortController, set a timer to call controller.abort() after LLM_TIMEOUT_MS,
pass controller.signal to anthropic.messages.create or to axios (signal option)
or reject the race on timeout, clear the timer on success, and surface a
consistent timeout error; apply this pattern to callAnthropic, callLLMRawText,
and any axios.post usages referenced (anthropic.messages.create / new Anthropic
/ axios.post).
- Around line 309-318: The code currently allows arbitrary config.baseUrl to
flow into server-side axios.post in callOpenAI (and similar call sites), so
enforce a server-managed allowlist and network-block checks before making
outbound requests: ensure config.baseUrl is only accepted if it matches an
explicit allowlist of approved origins (use a centralized ALLOWLIST and default
to the safe OpenAI URL when absent), parse the baseUrl with URL to extract
hostname, resolve the hostname to IP(s) and reject any addresses in private,
link-local, loopback, or metadata ranges (e.g., 127.0.0.0/8, 10.0.0.0/8,
172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16, 169.254.169.254, ::1, fc00::/7,
fe80::/10), and also block hostnames that resolve to such IPs; if validation
fails, throw a clear error and do not call axios.post; apply this validation in
callOpenAI and the other LLM call functions (the same pattern used around
config.baseUrl) so only approved, non-private endpoints are contacted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6862314-0a8e-487e-afd3-dad5e1a07983
📒 Files selected for processing (2)
server/src/sdk/browserAgent.tssrc/components/robot/pages/RobotCreate.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/robot/pages/RobotCreate.tsx
Comment on lines
+88
to
+103
| function extractJson(content: string): AgentAction { | ||
| const cleaned = content.trim(); | ||
| const start = cleaned.indexOf('{'); | ||
| const end = cleaned.lastIndexOf('}'); | ||
| if (start === -1 || end === -1 || end <= start) { | ||
| return { action: 'done', result: cleaned || 'No result returned by LLM' }; | ||
| } | ||
| try { | ||
| const parsed = JSON.parse(cleaned.substring(start, end + 1)); | ||
| if (parsed.result !== undefined && typeof parsed.result !== 'string') { | ||
| parsed.result = JSON.stringify(parsed.result, null, 2); | ||
| } | ||
| return parsed; | ||
| } catch { | ||
| return { action: 'done', result: cleaned }; | ||
| } |
There was a problem hiding this comment.
Validate model output before treating it as executable or complete.
Malformed/non-JSON LLM output currently becomes a successful done result, and unsupported action names fall through executeAction as a no-op. Since this JSON is untrusted runtime data, reject invalid actions, enforce [data-agent-idx="N"] selectors, and restrict navigate targets before executing anything.
🛡️ Suggested validation shape
+const AGENT_SELECTOR_RE = /^\[data-agent-idx="\d+"\]$/;
+const VALID_ACTIONS = new Set([
+ 'click', 'type', 'navigate', 'scroll', 'wait', 'press', 'select', 'check', 'hover', 'done'
+]);
+
+function assertValidAction(action: any): asserts action is AgentAction {
+ if (!action || typeof action !== 'object' || !VALID_ACTIONS.has(action.action)) {
+ throw new Error(`Unsupported or malformed LLM action: ${JSON.stringify(action)}`);
+ }
+
+ if (action.selector && !AGENT_SELECTOR_RE.test(action.selector)) {
+ throw new Error(`Invalid selector "${action.selector}". Use [data-agent-idx="N"] from the element list.`);
+ }
+}
+
function extractJson(content: string): AgentAction {
const cleaned = content.trim();
const start = cleaned.indexOf('{');
const end = cleaned.lastIndexOf('}');
if (start === -1 || end === -1 || end <= start) {
- return { action: 'done', result: cleaned || 'No result returned by LLM' };
+ throw new Error(`LLM did not return JSON: ${cleaned.substring(0, 200)}`);
}
try {
const parsed = JSON.parse(cleaned.substring(start, end + 1));
if (parsed.result !== undefined && typeof parsed.result !== 'string') {
parsed.result = JSON.stringify(parsed.result, null, 2);
}
+ assertValidAction(parsed);
return parsed;
- } catch {
- return { action: 'done', result: cleaned };
+ } catch (err: any) {
+ throw new Error(`Invalid LLM JSON action: ${err.message}`);
}
}
async function executeAction(page: Page, action: AgentAction): Promise<void> {
+ assertValidAction(action);
switch (action.action) {Also applies to: 486-554
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/sdk/browserAgent.ts` around lines 88 - 103, extractJson currently
treats malformed or non-JSON LLM output as a successful "done" and executeAction
allows unknown/unsafe actions; change this to validate and reject untrusted
model output before executing: in extractJson and its callers (e.g.,
executeAction) ensure parsed JSON matches an explicit schema (allowed action
names like "click","type","navigate","done"), require the selector field to
match /^\[data-agent-idx="(\d+)"\]$/ and coerce/serialize non-string result
fields rather than silently accepting them, and for "navigate" actions restrict
targets to an allowlist of safe origins or relative paths; if validation fails,
return a standardized error action (not "done") and do not proceed to execution.
Use the function names extractJson and executeAction to locate the logic to
update.
Comment on lines
+513
to
+516
| case 'navigate': { | ||
| if (!action.url) throw new Error('navigate: missing url'); | ||
| await page.goto(action.url, { waitUntil: 'domcontentloaded', timeout: 30000 }); | ||
| break; |
There was a problem hiding this comment.
Constrain navigate to safe destinations.
The model can instruct the backend browser to navigate to arbitrary URLs, including internal or metadata endpoints, then capture and return their contents. Restrict navigation to the current origin or a server-managed allowlist, and reject non-HTTP(S), private, link-local, and metadata IP targets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/sdk/browserAgent.ts` around lines 513 - 516, The navigate handler
(case 'navigate' in server/src/sdk/browserAgent.ts) currently allows arbitrary
URLs; restrict it by validating action.url before calling page.goto: parse the
URL with new URL(action.url, currentPageUrl), require protocol === 'http:' ||
'https:', then either enforce same-origin (compare parsedUrl.origin to
currentPageUrl.origin) or check parsedUrl.origin/hostname against a
server-managed allowlist function (e.g., isAllowedOrigin/allowlist). Also reject
hostnames or resolved IPs in private, loopback, link-local or cloud metadata
ranges (e.g., 127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16,
169.254.0.0/16, fe80::/10, and 169.254.169.254), using DNS resolution/net.isIP
to detect numeric addresses and blacklist ranges, and throw an error if
validation fails instead of calling page.goto.
Comment on lines
+536
to
+539
| case 'check': { | ||
| if (!action.selector) throw new Error('check: missing selector'); | ||
| await page.click(action.selector, { timeout: 5000 }); | ||
| break; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current Playwright checkbox/toggle handling.
rg -n 'case '\''check'\''|page\.check|setChecked|aria-checked|page\.click\(action\.selector' -C 4 server/src/sdk/browserAgent.tsRepository: getmaxun/maxun
Length of output: 836
🏁 Script executed:
# Check Playwright's page.check() API and verify if it exists and is appropriate
# Also search for any other 'check' action usages or similar patterns
rg -n "page\.check\(|page\.uncheck\(|action\.type.*check|'check'" server/src/sdk/browserAgent.tsRepository: getmaxun/maxun
Length of output: 195
🌐 Web query:
Playwright page.check() API behavior idempotent
💡 Result:
Yes, the Playwright locator.check API (used as page.check(selector) or locator.check) is idempotent. It ensures the checkbox or radio element is checked by performing actionability checks, scrolling into view if needed, and dispatching events only if the element is not already checked. If already checked, it skips the interaction step and succeeds without error or side effects like unchecking.
Citations:
- 1: https://playwright.dev/docs/next/api/class-locator
- 2: https://playwright.dev/docs/api/class-locator
- 3: https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/client/locator.ts
🏁 Script executed:
# Search for tests or other files that might reference the 'check' action
fd -e ts -e js | xargs grep -l "action.*check\|'check'" | head -20Repository: getmaxun/maxun
Length of output: 223
🏁 Script executed:
# Also verify if there are any other action handlers that might have similar issues
rg -n "case '(click|toggle|check|select)'" server/src/sdk/browserAgent.ts | head -20Repository: getmaxun/maxun
Length of output: 130
Replace click() with page.check() to make the check action idempotent.
Using click() toggles the checkbox instead of ensuring it's checked. Playwright's page.check() API is idempotent—it verifies actionability and checks the element only if not already checked.
Suggested fix
case 'check': {
if (!action.selector) throw new Error('check: missing selector');
- await page.click(action.selector, { timeout: 5000 });
+ await page.check(action.selector, { timeout: 5000 }).catch(async () => {
+ const locator = page.locator(action.selector!);
+ const ariaChecked = await locator.getAttribute('aria-checked').catch(() => null);
+ if (ariaChecked !== 'true') {
+ await locator.click({ timeout: 5000 });
+ }
+ });
break;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/sdk/browserAgent.ts` around lines 536 - 539, In the 'check' case
inside the switch handling browser actions (the block that currently throws
"check: missing selector" and calls page.click), replace the use of
page.click(action.selector, { timeout: 5000 }) with page.check(action.selector,
{ timeout: 5000 }) so the operation is idempotent and only checks the element if
not already checked; keep the existing selector validation and timeout behavior
around the page.check call.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/run/RunContent.tsx (1)
97-130:⚠️ Potential issue | 🟠 MajorReset
textContentwhen loading a new output.
textContentis populated fromrow.serializableOutput, but unlike markdown/html/smart-query it is never cleared. Opening a run without text after one with text can render/download the previous run’s content.🐛 Proposed fix
useEffect(() => { setMarkdownContent(''); setHtmlContent(''); + setTextContent(''); setSmartQueryResult('');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/run/RunContent.tsx` around lines 97 - 130, The effect that reads row.serializableOutput never clears previous text content; modify the useEffect handling (the function using setTextContent and reading row.serializableOutput) to explicitly reset textContent at the start (like setTextContent('')) and ensure that if no textOutput is found it remains cleared; update the logic around textOutput (in the same effect) so every branch either sets text via setTextContent(...) or leaves it cleared, referencing the existing symbols useEffect, setTextContent, textOutput, and row.serializableOutput.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/run/RunContent.tsx`:
- Around line 1096-1101: The "has output" boolean (hasData/hasScreenshots) is
missing smart-query results, causing smart-query-only runs to show the
empty-output message; update the checks that determine whether output exists
(e.g., the hasData initializer and any other "has output" guards around lines
where empty-output is decided) to include smartQueryResult.length > 0 (and where
appropriate consider hasSmartQuery) so smart-query-only results are treated as
output; apply the same change to the other occurrences noted (the other
hasData/hasScreenshots checks referenced).
---
Outside diff comments:
In `@src/components/run/RunContent.tsx`:
- Around line 97-130: The effect that reads row.serializableOutput never clears
previous text content; modify the useEffect handling (the function using
setTextContent and reading row.serializableOutput) to explicitly reset
textContent at the start (like setTextContent('')) and ensure that if no
textOutput is found it remains cleared; update the logic around textOutput (in
the same effect) so every branch either sets text via setTextContent(...) or
leaves it cleared, referencing the existing symbols useEffect, setTextContent,
textOutput, and row.serializableOutput.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ffd33ae-a31c-4f78-92d1-b110acad8f6d
📒 Files selected for processing (1)
src/components/run/RunContent.tsx
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/run/RunContent.tsx (1)
97-130:⚠️ Potential issue | 🟡 MinorReset
textContentwhen the run output changes.The effect clears markdown/HTML/smart-query state but leaves
textContentfrom the previous row. Switching from a text-output run to a run without text will show stale “Text Content”.🐛 Proposed fix
useEffect(() => { setMarkdownContent(''); setHtmlContent(''); + setTextContent(''); setSmartQueryResult('');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/run/RunContent.tsx` around lines 97 - 130, The effect in RunContent (useEffect in RunContent.tsx) resets markdown/html/smart-query but never clears textContent, causing stale text to persist; add a call to setTextContent('') at the top of the effect (alongside setMarkdownContent('') and setHtmlContent('')) so textContent is cleared whenever row changes, and also ensure that when textOutput is absent or not matched by the parsing branches you explicitly call setTextContent('') to avoid leaving previous content.
♻️ Duplicate comments (1)
src/components/run/RunContent.tsx (1)
1098-1103:⚠️ Potential issue | 🟡 MinorInclude smart-query results in the empty-output guard.
Smart-query-only runs still satisfy
!hasData && !hasScreenshots, so the empty-output message renders even though the smart-query accordion is displayed below.🐛 Proposed fix
const hasMarkdown = markdownContent && markdownContent.length > 0; const hasHTML = htmlContent && htmlContent.length > 0; const hasTextFormat = textContent && textContent.length > 0; const hasSmartQuery = smartQueryResult.length > 0; + const hasDisplayableOutput = + hasData || + hasScreenshots || + hasMarkdown || + hasHTML || + hasTextFormat || + hasSmartQuery;- ) : (!hasData && !hasScreenshots + ) : (!hasDisplayableOutput ? (['failed', 'aborted', 'aborting'].includes(row.status)Also applies to: 1242-1246, 2275-2309
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/run/RunContent.tsx` around lines 1098 - 1103, The empty-output guard currently checks hasData and hasScreenshots (variables defined as hasData, hasScreenshots, hasMarkdown, hasHTML, hasTextFormat) but omits hasSmartQuery (derived from smartQueryResult), causing smart-query-only runs to show the empty message; update the empty-output conditional(s) in RunContent.tsx to include hasSmartQuery (e.g., && !hasSmartQuery or incorporate it into the existing OR chain) wherever the empty-output message is rendered (the same pattern near the other guards that use hasData/hasScreenshots — update those similar conditionals as well) so that smartQueryResult presence prevents the empty-output message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/api/record.ts`:
- Around line 810-827: executeBrowserAgent(...) can hang and must be bounded by
the existing SCRAPE_TIMEOUT; wrap the call to executeBrowserAgent(currentPage,
promptInstructions, llmConfig) with the helper withTimeout(...) using a sensible
timeout (e.g., SCRAPE_TIMEOUT or a shorter agent-specific constant) and await
the timed promise, so the agent cannot block the scrape run; on timeout
rejection catch it in the existing try/catch and set
serializableOutput.promptResult to a clear timeout message and log via logger
(use the same run id context), ensuring steps is an empty array to match current
error handling.
- Around line 1411-1414: Validate incoming smart-query inputs before calling
handleRunRecording: ensure requestedFormats (from req.body?.formats) is either
undefined/null or an array of strings (reject non-arrays or arrays containing
non-string entries) and ensure promptInstructions (from
req.body?.promptInstructions) is either undefined/null or a string (reject other
types); if validation fails, respond with a 400 error and a clear message and do
not call handleRunRecording; update the logic around runSource/runId to only
call handleRunRecording when inputs pass validation and reference the existing
variables requestedFormats, promptInstructions, runSource and the function
handleRunRecording to locate where to add these checks.
In `@server/src/api/sdk.ts`:
- Around line 155-159: The code is persisting raw LLM secrets (e.g.,
promptLlmApiKey and similar promptLlmBaseUrl) into the recording_meta returned
by the SDK; change the logic that builds recording_meta (where
promptLlmProvider, promptLlmModel, promptLlmApiKey, promptLlmBaseUrl are spread)
so you never store raw secrets: store only an encrypted/opaque secret reference
or key ID for promptLlmApiKey and any sensitive endpoint tokens, and ensure any
sensitive fields are redacted from the object returned by the SDK and from
analytics payloads (the same fix must be applied to the second block mentioned
around lines 171-181) before emitting or logging.
- Around line 502-505: The code is passing raw req.body values
(promptInstructions, formats) into interpreterSettings via handleRunRecording
and onward to executeBrowserAgent without validation; add runtime
validation/sanitization before calling handleRunRecording: ensure
promptInstructions is either undefined or a string (coerce toString or reject
with 400), ensure requestedFormats is either undefined or an array of strings
and filter/validate each entry against the allowed formats list/enum (drop
invalid entries or return 400), then pass the validated/sanitized values into
handleRunRecording and into interpreterSettings/executeBrowserAgent so no
non-string or invalid-format values can reach those functions.
---
Outside diff comments:
In `@src/components/run/RunContent.tsx`:
- Around line 97-130: The effect in RunContent (useEffect in RunContent.tsx)
resets markdown/html/smart-query but never clears textContent, causing stale
text to persist; add a call to setTextContent('') at the top of the effect
(alongside setMarkdownContent('') and setHtmlContent('')) so textContent is
cleared whenever row changes, and also ensure that when textOutput is absent or
not matched by the parsing branches you explicitly call setTextContent('') to
avoid leaving previous content.
---
Duplicate comments:
In `@src/components/run/RunContent.tsx`:
- Around line 1098-1103: The empty-output guard currently checks hasData and
hasScreenshots (variables defined as hasData, hasScreenshots, hasMarkdown,
hasHTML, hasTextFormat) but omits hasSmartQuery (derived from smartQueryResult),
causing smart-query-only runs to show the empty message; update the empty-output
conditional(s) in RunContent.tsx to include hasSmartQuery (e.g., &&
!hasSmartQuery or incorporate it into the existing OR chain) wherever the
empty-output message is rendered (the same pattern near the other guards that
use hasData/hasScreenshots — update those similar conditionals as well) so that
smartQueryResult presence prevents the empty-output message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c7e48fb-11cd-4ff9-a3a1-4e052914850a
📒 Files selected for processing (7)
server/src/api/record.tsserver/src/api/sdk.tsserver/src/mcp-worker.tsserver/src/models/Run.tsserver/src/pgboss-worker.tsserver/src/workflow-management/scheduler/index.tssrc/components/run/RunContent.tsx
✅ Files skipped from review due to trivial changes (1)
- server/src/models/Run.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- server/src/workflow-management/scheduler/index.ts
- server/src/pgboss-worker.ts
Comment on lines
+810
to
+827
| const promptInstructions = promptInstructionsOverride || (recording.recording_meta as any).promptInstructions as string | undefined; | ||
| if (promptInstructions && currentPage) { | ||
| try { | ||
| const llmConfig = { | ||
| provider: ((recording.recording_meta as any).promptLlmProvider || 'ollama') as 'anthropic' | 'openai' | 'ollama', | ||
| model: (recording.recording_meta as any).promptLlmModel as string | undefined, | ||
| apiKey: (recording.recording_meta as any).promptLlmApiKey as string | undefined, | ||
| baseUrl: (recording.recording_meta as any).promptLlmBaseUrl as string | undefined, | ||
| }; | ||
| logger.log('info', `Running smart query for API scrape run ${plainRun.runId}`); | ||
| const agentResult = await executeBrowserAgent(currentPage, promptInstructions, llmConfig); | ||
| serializableOutput.promptResult = [{ content: agentResult.result, steps: agentResult.steps }]; | ||
| logger.log('info', `Smart query completed for API scrape run ${plainRun.runId}`); | ||
| } catch (agentErr: any) { | ||
| logger.log('warn', `Smart query failed for API scrape run ${plainRun.runId}: ${agentErr.message}`); | ||
| serializableOutput.promptResult = [{ content: `Smart query failed: ${agentErr.message}`, steps: [] }]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Bound smart-query execution with a timeout.
executeBrowserAgent(...) can block this scrape run indefinitely relative to the conversion steps, which already use SCRAPE_TIMEOUT. Wrap the agent call with withTimeout so a stuck LLM/browser loop doesn’t hold the run/browser until the outer 3-hour poll timeout.
🛡️ Proposed fix
+ const SMART_QUERY_TIMEOUT = 120000;
const promptInstructions = promptInstructionsOverride || (recording.recording_meta as any).promptInstructions as string | undefined;
if (promptInstructions && currentPage) {
try {
const llmConfig = {
provider: ((recording.recording_meta as any).promptLlmProvider || 'ollama') as 'anthropic' | 'openai' | 'ollama',
model: (recording.recording_meta as any).promptLlmModel as string | undefined,
apiKey: (recording.recording_meta as any).promptLlmApiKey as string | undefined,
baseUrl: (recording.recording_meta as any).promptLlmBaseUrl as string | undefined,
};
logger.log('info', `Running smart query for API scrape run ${plainRun.runId}`);
- const agentResult = await executeBrowserAgent(currentPage, promptInstructions, llmConfig);
+ const agentResult = await withTimeout(
+ executeBrowserAgent(currentPage, promptInstructions, llmConfig),
+ SMART_QUERY_TIMEOUT,
+ 'Smart query'
+ );
serializableOutput.promptResult = [{ content: agentResult.result, steps: agentResult.steps }];
logger.log('info', `Smart query completed for API scrape run ${plainRun.runId}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/api/record.ts` around lines 810 - 827, executeBrowserAgent(...)
can hang and must be bounded by the existing SCRAPE_TIMEOUT; wrap the call to
executeBrowserAgent(currentPage, promptInstructions, llmConfig) with the helper
withTimeout(...) using a sensible timeout (e.g., SCRAPE_TIMEOUT or a shorter
agent-specific constant) and await the timed promise, so the agent cannot block
the scrape run; on timeout rejection catch it in the existing try/catch and set
serializableOutput.promptResult to a clear timeout message and log via logger
(use the same run id context), ensuring steps is an empty array to match current
error handling.
Comment on lines
1411
to
+1414
| const requestedFormats = req.body?.formats; | ||
| const promptInstructions = req.body?.promptInstructions; | ||
| const runSource = req.headers['x-run-source'] === 'mcp' ? 'mcp' : 'api'; | ||
| const runId = await handleRunRecording(req.params.id, req.user.id, runSource, requestedFormats); | ||
| const runId = await handleRunRecording(req.params.id, req.user.id, runSource, requestedFormats, promptInstructions); |
There was a problem hiding this comment.
Validate smart-query inputs before persisting the run.
formats and promptInstructions come directly from the request body. Non-array/invalid formats are silently ignored or can produce an empty successful run after filtering, and non-string promptInstructions can be passed to the browser agent at runtime. Reject invalid shapes here before calling handleRunRecording.
🐛 Proposed fix
const requestedFormats = req.body?.formats;
const promptInstructions = req.body?.promptInstructions;
+
+ if (requestedFormats !== undefined && !Array.isArray(requestedFormats)) {
+ return res.status(400).json({
+ statusCode: 400,
+ messageCode: "bad_request",
+ message: "formats must be an array of strings",
+ });
+ }
+
+ if (
+ requestedFormats?.some((format: unknown) =>
+ typeof format !== 'string' ||
+ !['markdown', 'html', 'screenshot-visible', 'screenshot-fullpage'].includes(format)
+ )
+ ) {
+ return res.status(400).json({
+ statusCode: 400,
+ messageCode: "bad_request",
+ message: "formats contains unsupported values",
+ });
+ }
+
+ if (promptInstructions !== undefined && typeof promptInstructions !== 'string') {
+ return res.status(400).json({
+ statusCode: 400,
+ messageCode: "bad_request",
+ message: "promptInstructions must be a string",
+ });
+ }
+
const runSource = req.headers['x-run-source'] === 'mcp' ? 'mcp' : 'api';
const runId = await handleRunRecording(req.params.id, req.user.id, runSource, requestedFormats, promptInstructions);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/api/record.ts` around lines 1411 - 1414, Validate incoming
smart-query inputs before calling handleRunRecording: ensure requestedFormats
(from req.body?.formats) is either undefined/null or an array of strings (reject
non-arrays or arrays containing non-string entries) and ensure
promptInstructions (from req.body?.promptInstructions) is either undefined/null
or a string (reject other types); if validation fails, respond with a 400 error
and a clear message and do not call handleRunRecording; update the logic around
runSource/runId to only call handleRunRecording when inputs pass validation and
reference the existing variables requestedFormats, promptInstructions, runSource
and the function handleRunRecording to locate where to add these checks.
Comment on lines
+155
to
+159
| ...(promptInstructionsForMeta ? { promptInstructions: promptInstructionsForMeta } : {}), | ||
| ...((workflowFile.meta as any).promptLlmProvider ? { promptLlmProvider: (workflowFile.meta as any).promptLlmProvider } : {}), | ||
| ...((workflowFile.meta as any).promptLlmModel ? { promptLlmModel: (workflowFile.meta as any).promptLlmModel } : {}), | ||
| ...((workflowFile.meta as any).promptLlmApiKey ? { promptLlmApiKey: (workflowFile.meta as any).promptLlmApiKey } : {}), | ||
| ...((workflowFile.meta as any).promptLlmBaseUrl ? { promptLlmBaseUrl: (workflowFile.meta as any).promptLlmBaseUrl } : {}), |
There was a problem hiding this comment.
Do not store or emit raw LLM API keys in robot metadata.
Line 158 persists promptLlmApiKey into recording_meta; this object is returned from SDK robot APIs and included in the analytics payload at Line 175. Store an encrypted secret reference instead, and redact secrets from responses/telemetry.
Also applies to: 171-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/api/sdk.ts` around lines 155 - 159, The code is persisting raw LLM
secrets (e.g., promptLlmApiKey and similar promptLlmBaseUrl) into the
recording_meta returned by the SDK; change the logic that builds recording_meta
(where promptLlmProvider, promptLlmModel, promptLlmApiKey, promptLlmBaseUrl are
spread) so you never store raw secrets: store only an encrypted/opaque secret
reference or key ID for promptLlmApiKey and any sensitive endpoint tokens, and
ensure any sensitive fields are redacted from the object returned by the SDK and
from analytics payloads (the same fix must be applied to the second block
mentioned around lines 171-181) before emitting or logging.
Comment on lines
+502
to
+505
| const promptInstructions = req.body?.promptInstructions; | ||
| const requestedFormats = req.body?.formats; | ||
|
|
||
| const runId = await handleRunRecording(robotId, user.id.toString(), runSource, requestedFormats, promptInstructions); |
There was a problem hiding this comment.
Validate execute-time prompt and format overrides.
This endpoint forwards raw request body values into interpreterSettings. Invalid formats can be silently dropped later, and non-string promptInstructions can reach executeBrowserAgent as the wrong runtime type.
🐛 Proposed fix
const runSource = req.headers['x-run-source'] === 'cli' ? 'cli' : 'sdk';
const promptInstructions = req.body?.promptInstructions;
- const requestedFormats = req.body?.formats;
+ if (promptInstructions !== undefined && typeof promptInstructions !== 'string') {
+ return res.status(400).json({
+ error: "promptInstructions must be a string"
+ });
+ }
+
+ const { validFormats: requestedFormats, invalidFormats } = parseOutputFormats(
+ req.body?.formats,
+ SCRAPE_OUTPUT_FORMAT_OPTIONS
+ );
+ if (invalidFormats.length > 0) {
+ return res.status(400).json({
+ error: `Invalid formats: ${invalidFormats.map(String).join(', ')}`
+ });
+ }
const runId = await handleRunRecording(robotId, user.id.toString(), runSource, requestedFormats, promptInstructions);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/api/sdk.ts` around lines 502 - 505, The code is passing raw
req.body values (promptInstructions, formats) into interpreterSettings via
handleRunRecording and onward to executeBrowserAgent without validation; add
runtime validation/sanitization before calling handleRunRecording: ensure
promptInstructions is either undefined or a string (coerce toString or reject
with 400), ensure requestedFormats is either undefined or an array of strings
and filter/validate each entry against the allowed formats list/enum (drop
invalid entries or return 400), then pass the validated/sanitized values into
handleRunRecording and into interpreterSettings/executeBrowserAgent so no
non-string or invalid-format values can reach those functions.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/routes/storage.ts (1)
528-591:⚠️ Potential issue | 🟠 MajorEncrypt
promptLlmApiKeyat rest and validate the new prompt fields.
promptLlmApiKeyis accepted from the request body and persisted verbatim intorecording_meta. All downstream readers (scheduler/index.ts,api/record.ts,pgboss-worker.ts) then hand it straight toexecuteBrowserAgentwithout decryption, meaning user API keys are stored in plaintext in the DB. This file already importsencrypt/decrypt; store the value viaencrypt(...)at write time and decrypt on read at each consumer (or, at minimum, redact it from logs/analytics). Note thatcapture('maxun-oss-robot-created', { robot_meta: newRobot.recording_meta, ... })on Line 603–609 will ship the raw key off to analytics.- None of the new fields are validated before being written:
- Non-string
promptInstructionswill be coerced (e.g.String({x:1}).substring(0,1000)→"[object Object]").promptLlmProvideris typed inRobotMetaas'anthropic' | 'openai' | 'ollama'but any string is accepted.promptLlmBaseUrlis not checked to be a valid URL.PUT /recordings/:id(Line 338+) does not allow updating/clearing any of these new fields, so there is no path to rotate the stored API key without recreating the robot.Consider validating types/enums up front, encrypting
promptLlmApiKey, and extending the PUT handler so the prompt config (especially the API key) can be updated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/routes/storage.ts` around lines 528 - 591, The new prompt fields are persisted without validation and the promptLlmApiKey is stored in plaintext; modify the Robot.create flow in storage.ts to validate types/enums before persisting (ensure promptInstructions is a string and truncate, ensure promptLlmProvider is one of 'anthropic'|'openai'|'ollama', validate promptLlmBaseUrl is a well-formed URL) and encrypt promptLlmApiKey using the existing encrypt(...) import before saving (decrypt(...) when consumers like scheduler/index.ts, api/record.ts, pgboss-worker.ts call executeBrowserAgent), update the PUT /recordings/:id handler to permit updating/clearing the prompt fields (so keys can be rotated), and redact or omit the raw promptLlmApiKey from the capture('maxun-oss-robot-created', { robot_meta: newRobot.recording_meta, ... }) payload to avoid shipping secrets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/workflow-management/scheduler/index.ts`:
- Around line 344-364: The outer guard currently checks
recording.recording_meta.promptInstructions but the inner redeclaration of
promptInstructions shadows it and prevents
run.interpreterSettings?.promptInstructions from ever taking effect; fix by
computing the effective promptInstructions once (e.g., const
effectivePromptInstructions = run.interpreterSettings?.promptInstructions ||
(recording.recording_meta as any).promptInstructions) before the if, replace
both uses of promptInstructions with that effective variable, and then gate the
whole block on effectivePromptInstructions so executeBrowserAgent,
serializableOutput.promptResult and the logging can run correctly when
interpreterSettings provides an override.
- Around line 354-363: Wrap the call to executeBrowserAgent with the existing
withTimeout helper using the SCRAPE_TIMEOUT constant so the smart-query cannot
block forever; replace await executeBrowserAgent(currentPage,
promptInstructions, llmConfig) with await withTimeout(() =>
executeBrowserAgent(currentPage, promptInstructions, llmConfig), SCRAPE_TIMEOUT)
(or equivalent promise form) and ensure timeout rejections are caught by the
existing catch so serializableOutput.promptResult is set to a timeout message
(keeping the same shape used elsewhere), referencing the surrounding variables
run.interpreterSettings, promptInstructions, serializableOutput.promptResult and
logger for context.
---
Outside diff comments:
In `@server/src/routes/storage.ts`:
- Around line 528-591: The new prompt fields are persisted without validation
and the promptLlmApiKey is stored in plaintext; modify the Robot.create flow in
storage.ts to validate types/enums before persisting (ensure promptInstructions
is a string and truncate, ensure promptLlmProvider is one of
'anthropic'|'openai'|'ollama', validate promptLlmBaseUrl is a well-formed URL)
and encrypt promptLlmApiKey using the existing encrypt(...) import before saving
(decrypt(...) when consumers like scheduler/index.ts, api/record.ts,
pgboss-worker.ts call executeBrowserAgent), update the PUT /recordings/:id
handler to permit updating/clearing the prompt fields (so keys can be rotated),
and redact or omit the raw promptLlmApiKey from the
capture('maxun-oss-robot-created', { robot_meta: newRobot.recording_meta, ... })
payload to avoid shipping secrets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93163fb3-08d0-497a-9f06-96bc75e70599
📒 Files selected for processing (5)
server/src/api/record.tsserver/src/api/sdk.tsserver/src/pgboss-worker.tsserver/src/routes/storage.tsserver/src/workflow-management/scheduler/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- server/src/pgboss-worker.ts
- server/src/api/sdk.ts
Comment on lines
+344
to
+364
| const promptInstructions = (recording.recording_meta as any).promptInstructions as string | undefined; | ||
| if (promptInstructions && currentPage) { | ||
| try { | ||
| const llmConfig = { | ||
| provider: ((recording.recording_meta as any).promptLlmProvider || 'ollama') as 'anthropic' | 'openai' | 'ollama', | ||
| model: (recording.recording_meta as any).promptLlmModel as string | undefined, | ||
| apiKey: (recording.recording_meta as any).promptLlmApiKey as string | undefined, | ||
| baseUrl: (recording.recording_meta as any).promptLlmBaseUrl as string | undefined, | ||
| }; | ||
| logger.log('info', `Running smart query for scheduled scrape run ${plainRun.runId}`); | ||
| const promptInstructions = run.interpreterSettings?.promptInstructions || (recording.recording_meta as any).promptInstructions as string | undefined; | ||
| if (promptInstructions) { | ||
| const agentResult = await executeBrowserAgent(currentPage, promptInstructions, llmConfig); | ||
| serializableOutput.promptResult = [{ content: agentResult.result, steps: agentResult.steps }]; | ||
| logger.log('info', `Smart query completed for scheduled scrape run ${plainRun.runId}`); | ||
| } | ||
| } catch (agentErr: any) { | ||
| logger.log('warn', `Smart query failed for scheduled scrape run ${plainRun.runId}: ${agentErr.message}`); | ||
| serializableOutput.promptResult = [{ content: `Smart query failed: ${agentErr.message}`, steps: [] }]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Dead-code / shadowed promptInstructions makes the interpreterSettings override unreachable.
Line 344 guards the entire block on recording.recording_meta.promptInstructions only, then Line 354 redeclares promptInstructions (shadowing the outer one) as run.interpreterSettings?.promptInstructions || recording_meta.promptInstructions. Because the outer if at Line 345 has already required recording_meta.promptInstructions to be truthy, the run.interpreterSettings?.promptInstructions override path is unreachable from this entry, and the inner if (promptInstructions) at Line 355 is redundant.
Either drop the inner redeclaration, or (preferred, to mirror record.ts’s promptInstructionsOverride pattern) resolve the effective value once at the top and gate on that.
♻️ Proposed fix
- const promptInstructions = (recording.recording_meta as any).promptInstructions as string | undefined;
- if (promptInstructions && currentPage) {
+ const promptInstructionsOverride = run.interpreterSettings?.promptInstructions as string | undefined;
+ const promptInstructions = promptInstructionsOverride
+ || ((recording.recording_meta as any).promptInstructions as string | undefined);
+ if (promptInstructions && currentPage) {
try {
const llmConfig = {
provider: ((recording.recording_meta as any).promptLlmProvider || 'ollama') as 'anthropic' | 'openai' | 'ollama',
model: (recording.recording_meta as any).promptLlmModel as string | undefined,
apiKey: (recording.recording_meta as any).promptLlmApiKey as string | undefined,
baseUrl: (recording.recording_meta as any).promptLlmBaseUrl as string | undefined,
};
logger.log('info', `Running smart query for scheduled scrape run ${plainRun.runId}`);
- const promptInstructions = run.interpreterSettings?.promptInstructions || (recording.recording_meta as any).promptInstructions as string | undefined;
- if (promptInstructions) {
- const agentResult = await executeBrowserAgent(currentPage, promptInstructions, llmConfig);
- serializableOutput.promptResult = [{ content: agentResult.result, steps: agentResult.steps }];
- logger.log('info', `Smart query completed for scheduled scrape run ${plainRun.runId}`);
- }
+ const agentResult = await executeBrowserAgent(currentPage, promptInstructions, llmConfig);
+ serializableOutput.promptResult = [{ content: agentResult.result, steps: agentResult.steps }];
+ logger.log('info', `Smart query completed for scheduled scrape run ${plainRun.runId}`);
} catch (agentErr: any) {📝 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.
Suggested change
| const promptInstructions = (recording.recording_meta as any).promptInstructions as string | undefined; | |
| if (promptInstructions && currentPage) { | |
| try { | |
| const llmConfig = { | |
| provider: ((recording.recording_meta as any).promptLlmProvider || 'ollama') as 'anthropic' | 'openai' | 'ollama', | |
| model: (recording.recording_meta as any).promptLlmModel as string | undefined, | |
| apiKey: (recording.recording_meta as any).promptLlmApiKey as string | undefined, | |
| baseUrl: (recording.recording_meta as any).promptLlmBaseUrl as string | undefined, | |
| }; | |
| logger.log('info', `Running smart query for scheduled scrape run ${plainRun.runId}`); | |
| const promptInstructions = run.interpreterSettings?.promptInstructions || (recording.recording_meta as any).promptInstructions as string | undefined; | |
| if (promptInstructions) { | |
| const agentResult = await executeBrowserAgent(currentPage, promptInstructions, llmConfig); | |
| serializableOutput.promptResult = [{ content: agentResult.result, steps: agentResult.steps }]; | |
| logger.log('info', `Smart query completed for scheduled scrape run ${plainRun.runId}`); | |
| } | |
| } catch (agentErr: any) { | |
| logger.log('warn', `Smart query failed for scheduled scrape run ${plainRun.runId}: ${agentErr.message}`); | |
| serializableOutput.promptResult = [{ content: `Smart query failed: ${agentErr.message}`, steps: [] }]; | |
| } | |
| } | |
| const promptInstructionsOverride = run.interpreterSettings?.promptInstructions as string | undefined; | |
| const promptInstructions = promptInstructionsOverride | |
| || ((recording.recording_meta as any).promptInstructions as string | undefined); | |
| if (promptInstructions && currentPage) { | |
| try { | |
| const llmConfig = { | |
| provider: ((recording.recording_meta as any).promptLlmProvider || 'ollama') as 'anthropic' | 'openai' | 'ollama', | |
| model: (recording.recording_meta as any).promptLlmModel as string | undefined, | |
| apiKey: (recording.recording_meta as any).promptLlmApiKey as string | undefined, | |
| baseUrl: (recording.recording_meta as any).promptLlmBaseUrl as string | undefined, | |
| }; | |
| logger.log('info', `Running smart query for scheduled scrape run ${plainRun.runId}`); | |
| const agentResult = await executeBrowserAgent(currentPage, promptInstructions, llmConfig); | |
| serializableOutput.promptResult = [{ content: agentResult.result, steps: agentResult.steps }]; | |
| logger.log('info', `Smart query completed for scheduled scrape run ${plainRun.runId}`); | |
| } catch (agentErr: any) { | |
| logger.log('warn', `Smart query failed for scheduled scrape run ${plainRun.runId}: ${agentErr.message}`); | |
| serializableOutput.promptResult = [{ content: `Smart query failed: ${agentErr.message}`, steps: [] }]; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/workflow-management/scheduler/index.ts` around lines 344 - 364,
The outer guard currently checks recording.recording_meta.promptInstructions but
the inner redeclaration of promptInstructions shadows it and prevents
run.interpreterSettings?.promptInstructions from ever taking effect; fix by
computing the effective promptInstructions once (e.g., const
effectivePromptInstructions = run.interpreterSettings?.promptInstructions ||
(recording.recording_meta as any).promptInstructions) before the if, replace
both uses of promptInstructions with that effective variable, and then gate the
whole block on effectivePromptInstructions so executeBrowserAgent,
serializableOutput.promptResult and the logging can run correctly when
interpreterSettings provides an override.
Comment on lines
+354
to
+363
| const promptInstructions = run.interpreterSettings?.promptInstructions || (recording.recording_meta as any).promptInstructions as string | undefined; | ||
| if (promptInstructions) { | ||
| const agentResult = await executeBrowserAgent(currentPage, promptInstructions, llmConfig); | ||
| serializableOutput.promptResult = [{ content: agentResult.result, steps: agentResult.steps }]; | ||
| logger.log('info', `Smart query completed for scheduled scrape run ${plainRun.runId}`); | ||
| } | ||
| } catch (agentErr: any) { | ||
| logger.log('warn', `Smart query failed for scheduled scrape run ${plainRun.runId}: ${agentErr.message}`); | ||
| serializableOutput.promptResult = [{ content: `Smart query failed: ${agentErr.message}`, steps: [] }]; | ||
| } |
There was a problem hiding this comment.
Bound executeBrowserAgent with a timeout.
The scrape conversion steps above all use SCRAPE_TIMEOUT (120s), but the smart-query step can block indefinitely on a slow/stuck LLM or browser loop. This file already defines withTimeout (Line 126) — wrap the agent call the same way so the run is released and the catch below records a timeout promptResult.
🛡️ Proposed fix
+ const SMART_QUERY_TIMEOUT = 120000;
- const agentResult = await executeBrowserAgent(currentPage, promptInstructions, llmConfig);
+ const agentResult = await withTimeout(
+ executeBrowserAgent(currentPage, promptInstructions, llmConfig),
+ SMART_QUERY_TIMEOUT,
+ 'Smart query'
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/workflow-management/scheduler/index.ts` around lines 354 - 363,
Wrap the call to executeBrowserAgent with the existing withTimeout helper using
the SCRAPE_TIMEOUT constant so the smart-query cannot block forever; replace
await executeBrowserAgent(currentPage, promptInstructions, llmConfig) with await
withTimeout(() => executeBrowserAgent(currentPage, promptInstructions,
llmConfig), SCRAPE_TIMEOUT) (or equivalent promise form) and ensure timeout
rejections are caught by the existing catch so serializableOutput.promptResult
is set to a timeout message (keeping the same shape used elsewhere), referencing
the surrounding variables run.interpreterSettings, promptInstructions,
serializableOutput.promptResult and logger for context.
amhsirak
approved these changes
Apr 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does?
Allows configuration of additional instructions when creating a Scrape robot. AI Agent picks up these instructions and performs specified action in the browser to return the result.
Summary by CodeRabbit
New Features
UI
API
Reliability