Fix: Check for closed context during agent streaming#1538
Fix: Check for closed context during agent streaming#1538kernel-systems-bot wants to merge 1 commit intobrowserbase:mainfrom
Conversation
Adds ensureNotClosed() checks to V3AgentHandler to properly detect when stagehand.close() is called during agent execution. This mirrors the existing pattern from V3CuaAgentHandler. Before: Accessing null context threw TypeError with confusing message After: Throws StagehandClosedError with clear "Stagehand session was closed" Checks added in: - prepareAgent(): Before accessing context for initial page URL - createStepHandler(): At the start of each step callback - captureAndEmitScreenshot(): Before accessing context for screenshots
|
Greptile SummaryThis PR adds proper closed context detection to Key Changes:
Issues Found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Agent as V3AgentHandler
participant Context as V3.context
participant LLM as LLMClient
User->>Agent: execute() or stream()
Agent->>Agent: prepareAgent()
Agent->>Agent: ensureNotClosed()
alt context is null
Agent-->>User: throw StagehandClosedError
else context exists
Agent->>Context: awaitActivePage()
Context-->>Agent: page with URL
Agent->>LLM: generateText/streamText()
loop Each step
LLM->>Agent: onStepFinish callback
Agent->>Agent: createStepHandler()
Agent->>Agent: ensureNotClosed()
alt context closed during execution
Agent-->>User: throw StagehandClosedError
else context still exists
Agent->>Context: awaitActivePage()
Context-->>Agent: page URL
opt EVALS mode
Agent->>Agent: captureAndEmitScreenshot()
Agent->>Agent: ensureNotClosed()
alt context closed
Agent-->>User: throw StagehandClosedError
else context exists
Agent->>Context: awaitActivePage()
Context-->>Agent: screenshot
end
end
end
end
Agent-->>User: return result
end
|
There was a problem hiding this comment.
Additional Comments (1)
-
packages/core/lib/v3/handlers/v3AgentHandler.ts, line 214 (link)logic: Potential race condition -
this.v3.contextcould be set to null between theensureNotClosed()check at line 168 and this access. Consider storing context in a local variable after the check or adding anotherensureNotClosed()call before this line.
2 files reviewed, 1 comment
Fix: Stream Continues After stagehand.close()
Summary
Adds
ensureNotClosed()checks toV3AgentHandlerto properly detect whenstagehand.close()is called during agent execution. This mirrors the existing pattern fromV3CuaAgentHandlerand ensures the agent terminates gracefully with a clear error instead of continuing to execute with a null context.Problem
In
v3AgentHandler.ts, there are no checks to detect if the stagehand instance was closed during streaming. Whenstagehand.close()is called mid-execution:The CUA handler (
v3CuaAgentHandler.ts) already has this pattern implemented correctly withensureNotClosed()checks.Solution
Added the
ensureNotClosed()method and checks at key points in the agent lifecycle:Checks added in:
prepareAgent()- Before accessing context for initial page URLcreateStepHandler()- At the start of each step callbackcaptureAndEmitScreenshot()- Before accessing context for screenshotsImpact
Error message improvement:
"Cannot read properties of null (reading 'awaitActivePage')""Stagehand session was closed"Test Plan
stream-close-cleanup.test.ts(7 test cases)v3CuaAgentHandler.tsFiles Changed
packages/core/lib/v3/handlers/v3AgentHandler.ts- AddedensureNotClosed()method and checkspackages/core/tests/stream-close-cleanup.test.ts- New regression test (7 test cases)Feedback? Email p0@kernel.dev
Summary by cubic
Fixes BUG-019 by stopping agent streams after stagehand.close(). Adds closed-context checks to V3AgentHandler so runs end cleanly with StagehandClosedError instead of a TypeError.
Written for commit 54bc3fc. Summary will update on new commits.