Skip to content

Conversation

@seanmcguire12
Copy link
Member

@seanmcguire12 seanmcguire12 commented Dec 2, 2025

why

what changed

test plan


Summary by cubic

Fixes act, extract, and observe to honor the timeout parameter by adding step-wise timeout guards with clear, specific timeout errors. Also wires v3 deterministic actions to use the same guard.

Why:

  • Promise.race timeouts let downstream steps keep running.
  • We need early, consistent aborts across snapshots, LLM calls, and action execution.

What:

  • Added createTimeoutGuard and specific errors: ActTimeoutError, ExtractTimeoutError, ObserveTimeoutError.
  • Enforced timeouts across:
    • Hybrid snapshot capture
    • LLM inference (act/extract/observe)
    • performUnderstudyMethod and self-heal retries
  • Refactored ActHandler:
    • actFromObserveResult → takeDeterministicAction (per-step timeout enforcement)
    • Helpers for action extraction and normalization; centralized metrics kept
  • Updated v3 to pass a timeout guard into takeDeterministicAction.
  • Kept behavior for shadow DOM and unsupported actions the same.

Test Plan:

  • act() with a small timeout throws ActTimeoutError; no action runs after timeout.
  • extract() with a small timeout throws ExtractTimeoutError; LLM and post-processing stop.
  • observe() with a small timeout throws ObserveTimeoutError; result is aborted.
  • act() two-step: timeout during step 2 throws ActTimeoutError; step 2 does not run.
  • Self-heal: timeout during fallback snapshot or inference throws; no retry action executes.
  • No-timeout path: act/extract/observe complete and metrics record.
  • Public error types export ActTimeoutError, ExtractTimeoutError, and ObserveTimeoutError.

Written for commit a92039c. Summary will update automatically on new commits.

@changeset-bot
Copy link

changeset-bot bot commented Dec 2, 2025

⚠️ No Changeset found

Latest commit: a92039c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@seanmcguire12 seanmcguire12 changed the base branch from main to seanmcguire/stg-1020-refactor-acthandlerts December 2, 2025 00:35
@seanmcguire12
Copy link
Member Author

@greptileai

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 2, 2025

Greptile Overview

Greptile Summary

This PR fixes timeout handling in act(), extract(), and observe() methods by replacing the Promise.race pattern with step-wise timeout guards that check elapsed time before each major operation. The implementation introduces a createTimeoutGuard() utility that tracks elapsed time from start and throws specific timeout errors (ActTimeoutError, ExtractTimeoutError, ObserveTimeoutError) when the timeout threshold is exceeded.

Key Changes:

  • Created timeoutGuard.ts utility with time-based checking
  • Added specific timeout error classes for each method
  • Refactored act(), extract(), and observe() to call ensureTimeRemaining() before snapshots, LLM calls, and action execution
  • Updated takeDeterministicAction() to accept and propagate the timeout guard through self-heal retry paths
  • Ensured timeout errors are caught and rethrown in retry logic to prevent continuation after timeout

Confidence Score: 4/5

  • Safe to merge with thorough testing of timeout scenarios
  • The implementation is sound and addresses the core issue where timeouts were only enforced at the wrapper level. The step-wise timeout checks ensure operations abort early. Score reflects need to verify timeout behavior across all execution paths including self-heal retries and two-step actions
  • Pay close attention to packages/core/lib/v3/handlers/actHandler.ts due to complex two-step flow and self-heal retry logic with timeout handling

Important Files Changed

File Analysis

Filename Score Overview
packages/core/lib/v3/handlers/handlerUtils/timeoutGuard.ts 5/5 New utility creating timeout guards with start time tracking and optional error factories
packages/core/lib/v3/handlers/actHandler.ts 4/5 Refactored to enforce step-wise timeouts and catch timeout errors in self-heal retry logic
packages/core/lib/v3/handlers/extractHandler.ts 5/5 Replaced Promise.race pattern with step-wise timeout checks before snapshot and LLM calls
packages/core/lib/v3/handlers/observeHandler.ts 5/5 Replaced Promise.race pattern with step-wise timeout checks before snapshot and LLM calls

Sequence Diagram

sequenceDiagram
    participant User
    participant ActHandler
    participant TimeoutGuard
    participant Page
    participant LLMClient
    
    User->>ActHandler: act(instruction, {timeout: 5000})
    ActHandler->>TimeoutGuard: createTimeoutGuard(5000ms)
    TimeoutGuard-->>ActHandler: ensureTimeRemaining()
    
    ActHandler->>TimeoutGuard: ensureTimeRemaining()
    Note over TimeoutGuard: Check elapsed time < 5000ms
    
    ActHandler->>Page: waitForDomNetworkQuiet()
    Page-->>ActHandler: settled
    
    ActHandler->>TimeoutGuard: ensureTimeRemaining()
    ActHandler->>Page: captureHybridSnapshot()
    Page-->>ActHandler: {combinedTree, xpathMap}
    
    ActHandler->>TimeoutGuard: ensureTimeRemaining()
    ActHandler->>LLMClient: getActionFromLLM()
    LLMClient-->>ActHandler: {action, response}
    
    ActHandler->>TimeoutGuard: ensureTimeRemaining()
    ActHandler->>ActHandler: takeDeterministicAction(action, ensureTimeRemaining)
    
    ActHandler->>TimeoutGuard: ensureTimeRemaining()
    ActHandler->>Page: performUnderstudyMethod()
    
    alt Timeout Exceeded
        TimeoutGuard-->>ActHandler: throw ActTimeoutError
        ActHandler-->>User: ActTimeoutError
    else Success
        ActHandler-->>User: ActResult
    end
    
    alt Two-Step Flow
        ActHandler->>TimeoutGuard: ensureTimeRemaining()
        ActHandler->>Page: captureHybridSnapshot() (step 2)
        ActHandler->>TimeoutGuard: ensureTimeRemaining()
        ActHandler->>LLMClient: getActionFromLLM() (step 2)
        ActHandler->>TimeoutGuard: ensureTimeRemaining()
        ActHandler->>ActHandler: takeDeterministicAction(secondAction)
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants