Fix: Validate agent mode parameter #1535
Open
Conversation
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
Previously, passing an invalid mode (e.g., "garbage", "DOM", "") to stagehand.agent() would silently fall through to DOM mode behavior. This could confuse users who typo the mode or use wrong casing. Now throws StagehandInvalidArgumentError with a helpful message listing valid options: dom, hybrid, cua. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
Contributor
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/tests/invalid-agent-mode.test.ts">
<violation number="1" location="packages/core/tests/invalid-agent-mode.test.ts:80">
P2: Behavior tests reimplement validation locally and never exercise Stagehand.agent(), so regressions in agent() won’t be caught</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
| const mode = "dom"; | ||
|
|
||
| const isValid = validModes.includes(mode as any); |
Contributor
There was a problem hiding this comment.
P2: Behavior tests reimplement validation locally and never exercise Stagehand.agent(), so regressions in agent() won’t be caught
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/tests/invalid-agent-mode.test.ts, line 80:
<comment>Behavior tests reimplement validation locally and never exercise Stagehand.agent(), so regressions in agent() won’t be caught</comment>
<file context>
@@ -0,0 +1,163 @@
+ const validModes = ["dom", "hybrid", "cua"] as const;
+ const mode = "dom";
+
+ const isValid = validModes.includes(mode as any);
+ expect(isValid).toBe(true);
+ });
</file context>
Contributor
Greptile SummaryAdds validation to the Key changes:
Minor observations:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant V3 as V3.agent()
participant Validation as Mode Validator
participant Error as StagehandInvalidArgumentError
participant Agent as Agent Instance
User->>V3: agent({ mode: "invalid" })
V3->>V3: Check deprecated cua option
V3->>Validation: Validate mode parameter
Validation->>Validation: options?.mode !== undefined?
Validation->>Validation: mode in ["dom", "hybrid", "cua"]?
alt Invalid mode
Validation->>Error: Throw error with message
Error-->>User: "Invalid agent mode 'invalid'. Must be one of: dom, hybrid, cua"
else Valid mode or undefined
Validation->>V3: Continue
V3->>Agent: Create agent instance
Agent-->>User: Return agent
end
|
|
|
||
| // Validate mode parameter | ||
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
| if (options?.mode !== undefined && !validModes.includes(options.mode)) { |
Contributor
There was a problem hiding this comment.
syntax: Potential TypeScript strict mode issue: includes() on readonly tuple may not accept AgentToolMode without type assertion
Suggested change
| if (options?.mode !== undefined && !validModes.includes(options.mode)) { | |
| if (options?.mode !== undefined && !(validModes as readonly AgentToolMode[]).includes(options.mode)) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/lib/v3/v3.ts
Line: 1707:1707
Comment:
**syntax:** Potential TypeScript strict mode issue: `includes()` on readonly tuple may not accept `AgentToolMode` without type assertion
```suggestion
if (options?.mode !== undefined && !(validModes as readonly AgentToolMode[]).includes(options.mode)) {
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+66
to
+142
| describe("BUG-031: Agent mode validation behavior", () => { | ||
| let mockStagehand: any; | ||
|
|
||
| beforeEach(() => { | ||
| // Create a minimal mock that simulates the agent() validation path | ||
| // We can't fully initialize Stagehand without browser, so we test the pattern | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it("should accept valid mode 'dom'", async () => { | ||
| // This tests the validation logic pattern | ||
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
| const mode = "dom"; | ||
|
|
||
| const isValid = validModes.includes(mode as any); | ||
| expect(isValid).toBe(true); | ||
| }); | ||
|
|
||
| it("should accept valid mode 'hybrid'", async () => { | ||
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
| const mode = "hybrid"; | ||
|
|
||
| const isValid = validModes.includes(mode as any); | ||
| expect(isValid).toBe(true); | ||
| }); | ||
|
|
||
| it("should accept valid mode 'cua'", async () => { | ||
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
| const mode = "cua"; | ||
|
|
||
| const isValid = validModes.includes(mode as any); | ||
| expect(isValid).toBe(true); | ||
| }); | ||
|
|
||
| it("should reject invalid mode 'invalid'", async () => { | ||
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
| const mode = "invalid"; | ||
|
|
||
| const isValid = validModes.includes(mode as any); | ||
| expect(isValid).toBe(false); | ||
| }); | ||
|
|
||
| it("should reject case-sensitive variants like 'DOM', 'CUA', 'Hybrid'", async () => { | ||
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
|
|
||
| expect(validModes.includes("DOM" as any)).toBe(false); | ||
| expect(validModes.includes("CUA" as any)).toBe(false); | ||
| expect(validModes.includes("Hybrid" as any)).toBe(false); | ||
| expect(validModes.includes("HYBRID" as any)).toBe(false); | ||
| }); | ||
|
|
||
| it("should reject empty string", async () => { | ||
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
| const mode = ""; | ||
|
|
||
| const isValid = validModes.includes(mode as any); | ||
| expect(isValid).toBe(false); | ||
| }); | ||
|
|
||
| it("should reject typos like 'hybrd', 'doom', 'cua '", async () => { | ||
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
|
|
||
| expect(validModes.includes("hybrd" as any)).toBe(false); | ||
| expect(validModes.includes("doom" as any)).toBe(false); | ||
| expect(validModes.includes("cua " as any)).toBe(false); | ||
| expect(validModes.includes(" dom" as any)).toBe(false); | ||
| }); | ||
|
|
||
| it("should accept undefined mode (defaults to dom)", async () => { | ||
| // undefined mode should be allowed - it defaults to "dom" | ||
| const validModes = ["dom", "hybrid", "cua"] as const; | ||
| const mode = undefined; | ||
|
|
||
| // The validation only runs if mode is defined | ||
| const shouldValidate = mode !== undefined; | ||
| expect(shouldValidate).toBe(false); // No validation needed for undefined | ||
| }); |
Contributor
There was a problem hiding this comment.
style: Tests validate the logic pattern but don't actually call stagehand.agent() with invalid modes. Consider adding at least one integration test that verifies the error is thrown when calling the actual method, e.g.:
it("should throw error when agent() called with invalid mode", async () => {
const stagehand = new Stagehand({ env: "LOCAL" });
await stagehand.init();
await expect(async () => {
stagehand.agent({ mode: "invalid" as any });
}).rejects.toThrow(StagehandInvalidArgumentError);
await stagehand.close();
});Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/tests/invalid-agent-mode.test.ts
Line: 66:142
Comment:
**style:** Tests validate the logic pattern but don't actually call `stagehand.agent()` with invalid modes. Consider adding at least one integration test that verifies the error is thrown when calling the actual method, e.g.:
```typescript
it("should throw error when agent() called with invalid mode", async () => {
const stagehand = new Stagehand({ env: "LOCAL" });
await stagehand.init();
await expect(async () => {
stagehand.agent({ mode: "invalid" as any });
}).rejects.toThrow(StagehandInvalidArgumentError);
await stagehand.close();
});
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fix: Invalid Agent Mode Value Accepted
Summary
Adds validation for the
modeparameter instagehand.agent()to reject invalid values with a clear error message instead of silently falling back to DOM mode.Problem
The
agent()function accepted any value formodewithout validation:This could confuse users who:
"hybrd"instead of"hybrid")"DOM"instead of"dom")Solution
Added validation in
v3.tsagent()method that throwsStagehandInvalidArgumentErrorfor invalid modes:Behavior After Fix
Test Plan
invalid-agent-mode.test.ts(10 test cases)Files Changed
packages/core/lib/v3/v3.ts- Added mode validation inagent()methodpackages/core/tests/invalid-agent-mode.test.ts- New regression test (10 test cases)Feedback? Email [email protected]
Summary by cubic
Adds validation for stagehand.agent({ mode }) to reject invalid values with a clear error, instead of silently defaulting to DOM mode. Reduces confusion from typos and wrong casing.
Written for commit 06ef94f. Summary will update on new commits.