feat: add status check route sdk for Maxun CLI#1012
Merged
Conversation
WalkthroughAdded an authenticated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
🤖 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/sdk.ts`:
- Around line 39-44: The catch block in the status handler currently returns
internal error details to clients (catch (error: any) { logger.error(...);
return res.status(500).json({ error: "Failed to get status", message:
error.message }); }), which can leak internals; instead keep the full error in
server logs via logger.error (include error and error.stack) and return a
generic client-safe JSON (e.g., { error: "Failed to get status" } or a short
user-facing message) from res.status(500).json; update the catch block so
logger.error logs the full error and remove or replace usage of error.message in
the response.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dab1c36c-2de6-4977-8162-8e8d667e98d5
📒 Files selected for processing (1)
server/src/api/sdk.ts
Comment on lines
+39
to
+44
| } catch (error: any) { | ||
| logger.error("Error getting status:", error); | ||
| return res.status(500).json({ | ||
| error: "Failed to get status", | ||
| message: error.message | ||
| }); |
There was a problem hiding this comment.
Do not return raw internal error messages to clients.
On Line 43, message: error.message can expose internals in 500 responses. Keep detailed context in logs and return a generic client-safe message.
🔧 Suggested fix
} catch (error: any) {
logger.error("Error getting status:", error);
return res.status(500).json({
error: "Failed to get status",
- message: error.message
+ message: "Internal server error"
});
}📝 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
| } catch (error: any) { | |
| logger.error("Error getting status:", error); | |
| return res.status(500).json({ | |
| error: "Failed to get status", | |
| message: error.message | |
| }); | |
| } catch (error: any) { | |
| logger.error("Error getting status:", error); | |
| return res.status(500).json({ | |
| error: "Failed to get status", | |
| message: "Internal server error" | |
| }); | |
| } |
🤖 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 39 - 44, The catch block in the status
handler currently returns internal error details to clients (catch (error: any)
{ logger.error(...); return res.status(500).json({ error: "Failed to get
status", message: error.message }); }), which can leak internals; instead keep
the full error in server logs via logger.error (include error and error.stack)
and return a generic client-safe JSON (e.g., { error: "Failed to get status" }
or a short user-facing message) from res.status(500).json; update the catch
block so logger.error logs the full error and remove or replace usage of
error.message in the response.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/src/api/record.ts (1)
698-700: Reuse the resolvedrobotTypefor analytics too.This branch now honors legacy
recording_meta.robotType, but Line 1037 and Line 1182 still readrecording_meta.typedirectly forcapture(...). Records that only haverobotTypewill execute correctly while analytics still reportextract.🤖 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 698 - 700, The analytics still read recording.recording_meta.type directly even though you resolved robotType earlier; update the capture(...) calls (the ones that currently access recording.recording_meta.type) to use the already-resolved robotType variable instead so analytics honor legacy recording_meta.robotType as well—search for uses of recording.recording_meta.type near the capture(...) invocations and replace them with the robotType identifier so both behavior and analytics are consistent.
🤖 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/sdk.ts`:
- Around line 69-75: Validate and normalize workflowFile.meta.type/robotType
before persisting: instead of trusting the truthy value used by robotType =
(workflowFile.meta as any).type || (workflowFile.meta as any).robotType ||
'extract', explicitly check that the extracted value is a non-empty string and
one of the supported enum values (e.g., 'extract', 'scrape', ...), otherwise
either set a safe default ('extract') or reject/throw so invalid values like
"scrapper" or "" are not stored; update the logic around robotType and any other
similar usage (also at the later block around lines 102-109) to perform this
whitelist validation before saving recording_meta.type.
---
Nitpick comments:
In `@server/src/api/record.ts`:
- Around line 698-700: The analytics still read recording.recording_meta.type
directly even though you resolved robotType earlier; update the capture(...)
calls (the ones that currently access recording.recording_meta.type) to use the
already-resolved robotType variable instead so analytics honor legacy
recording_meta.robotType as well—search for uses of
recording.recording_meta.type near the capture(...) invocations and replace them
with the robotType identifier so both behavior and analytics are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7f75573-fdd9-4c6b-b8ff-ac1066c76914
📒 Files selected for processing (2)
server/src/api/record.tsserver/src/api/sdk.ts
Comment on lines
+69
to
75
| const robotType = (workflowFile.meta as any).type || (workflowFile.meta as any).robotType || 'extract'; | ||
|
|
||
| let enrichedWorkflow: any[] = []; | ||
| let extractedUrl: string | undefined; | ||
|
|
||
| if (type === 'scrape') { | ||
| if (robotType === 'scrape') { | ||
| enrichedWorkflow = []; |
There was a problem hiding this comment.
Reject unsupported robot types before persisting them.
recording_meta.type is treated as an enum elsewhere, but this path currently trusts any truthy meta.type / meta.robotType value and stores it. A typo like "scrapper" — or an empty string silently coerced by || — will be persisted as valid-looking metadata even though downstream execution only understands the supported set.
🔧 Proposed fix
- const robotType = (workflowFile.meta as any).type || (workflowFile.meta as any).robotType || 'extract';
+ const rawRobotType = (workflowFile.meta as any).type ?? (workflowFile.meta as any).robotType ?? 'extract';
+ const allowedRobotTypes = new Set(['extract', 'scrape', 'crawl', 'search']);
+
+ if (!allowedRobotTypes.has(rawRobotType)) {
+ return res.status(400).json({
+ error: "Invalid robot type. Must be one of: extract, scrape, crawl, search"
+ });
+ }
+
+ const robotType = rawRobotType as 'extract' | 'scrape' | 'crawl' | 'search';Also applies to: 102-109
🤖 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 69 - 75, Validate and normalize
workflowFile.meta.type/robotType before persisting: instead of trusting the
truthy value used by robotType = (workflowFile.meta as any).type ||
(workflowFile.meta as any).robotType || 'extract', explicitly check that the
extracted value is a non-empty string and one of the supported enum values
(e.g., 'extract', 'scrape', ...), otherwise either set a safe default
('extract') or reject/throw so invalid values like "scrapper" or "" are not
stored; update the logic around robotType and any other similar usage (also at
the later block around lines 102-109) to perform this whitelist validation
before saving recording_meta.type.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/src/api/sdk.ts (1)
858-860: Silent mode override may cause confusion.When
outputFormatsis provided, the user's explicitmode: 'discover'setting is silently overwritten to'scrape'. This could be unexpected. Consider either rejecting conflicting configurations or informing the user.💡 Option: reject conflicting configuration
+ if (searchConfig.mode === 'discover' && searchConfig.outputFormats && Array.isArray(searchConfig.outputFormats) && searchConfig.outputFormats.length > 0) { + return res.status(400).json({ + error: "outputFormats requires mode 'scrape'. Remove outputFormats or change mode to 'scrape'." + }); + } + if (searchConfig.outputFormats && Array.isArray(searchConfig.outputFormats) && searchConfig.outputFormats.length > 0) { searchConfig.mode = 'scrape'; }🤖 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 858 - 860, The code silently overwrites searchConfig.mode to 'scrape' whenever searchConfig.outputFormats is present, which can surprise callers who set mode: 'discover'; update the validation around searchConfig (checking searchConfig.outputFormats and searchConfig.mode) to either 1) throw a clear error rejecting the conflicting configuration (preferred) or 2) emit a visible warning/log and preserve the user's explicit mode rather than mutating it. Make the change where the current assignment exists (the block that sets searchConfig.mode = 'scrape') so it performs the conflict check and either throws/returns a descriptive error referencing outputFormats and mode, or calls the logger to warn and skips the silent override; also add/adjust unit tests to cover the conflicting-config behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/src/api/sdk.ts`:
- Around line 858-860: The code silently overwrites searchConfig.mode to
'scrape' whenever searchConfig.outputFormats is present, which can surprise
callers who set mode: 'discover'; update the validation around searchConfig
(checking searchConfig.outputFormats and searchConfig.mode) to either 1) throw a
clear error rejecting the conflicting configuration (preferred) or 2) emit a
visible warning/log and preserve the user's explicit mode rather than mutating
it. Make the change where the current assignment exists (the block that sets
searchConfig.mode = 'scrape') so it performs the conflict check and either
throws/returns a descriptive error referencing outputFormats and mode, or calls
the logger to warn and skips the silent override; also add/adjust unit tests to
cover the conflicting-config behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a09e1e1-30ec-40b7-950c-773552057214
📒 Files selected for processing (1)
server/src/api/sdk.ts
amhsirak
changed the title
amhsirak
approved these changes
Mar 24, 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?
Adds a status check route in sdk for Maxun CLI support.
Summary by CodeRabbit
New Features
Bug Fixes
Behavior Change