Skip to content

fix: return explicit status from full_reset to prevent silent skips (fixes #406) - #411

Open
Tharun-Varshan-S wants to merge 1 commit into
ServiceNow:mainfrom
Tharun-Varshan-S:fix-issue-406-full-reset-status
Open

Tharun-Varshan-S wants to merge 1 commit into
ServiceNow:mainfrom
Tharun-Varshan-S:fix-issue-406-full-reset-status

Conversation

@Tharun-Varshan-S

Copy link
Copy Markdown
Contributor

Description

Fixes #406

WebArenaInstance.full_reset() previously returned None in both scenarios:

  1. When the environment was successfully reset via the automated endpoint.
  2. When the reset was skipped because WA_FULL_RESET (or VWA_FULL_RESET) was unset and skip_if_not_set=True.

Because prepare_backend() called full_reset() without arguments, a missing reset endpoint only logged a single warning while returning None — exactly matching a successful reset. As a result, downstream benchmark runs could silently proceed against un-reset / dirty instances without callers having any programmatic signal to verify reset execution.

This PR implements the "fail legibly" approach:

  • WebArenaInstance.full_reset() now returns bool:
    • True if the automated reset was successfully executed.
    • False if the reset was skipped because the reset URL environment variable was unset or empty.
    • Continues raising exceptions when a configured reset endpoint fails or when skip_if_not_set=False.
  • Updated prepare_backend() at the call sites for webarena, webarena_verified, and visualwebarena to consume this return value and prominently log:
    • Environment reset: DONE (when True)
    • Environment reset: SKIPPED (<RESET_URL_VAR> unset) (when False)
  • Added comprehensive unit tests in tests/webarena/test_instance_reset.py covering all reset scenarios and log outputs.

What Changed

File Change
browsergym/webarena/src/browsergym/webarena/instance.py Updated full_reset() to track was_reset state, return bool, and updated docstrings.
browsergym/experiments/src/browsergym/experiments/benchmark/utils.py Updated prepare_backend() for webarena, webarena_verified, and visualwebarena to check full_reset() result and log reset status.
tests/webarena/test_instance_reset.py Added 12 unit tests verifying return values (True/False), exception handling on failure, VisualWebArena inheritance of VWA_FULL_RESET, and prepare_backend logging.

How It Was Tested

  1. Unit Tests:
    Ran uv run pytest -v tests/webarena/test_instance_reset.py — all 12 tests passed:

    • test_full_reset_skipped_when_env_unset
    • test_full_reset_skipped_when_env_empty
    • test_full_reset_raises_when_env_unset_and_skip_false
    • test_full_reset_raises_when_env_empty_and_skip_false
    • test_full_reset_success
    • test_full_reset_success_already_running
    • test_full_reset_raises_on_reset_endpoint_error
    • test_visualwebarena_uses_vwa_env_var
    • test_prepare_backend_webarena_logs_done
    • test_prepare_backend_webarena_logs_skipped
    • test_prepare_backend_visualwebarena_logs_skipped
    • test_prepare_backend_webarena_verified_logs_skipped
  2. Code Formatting & Linting:
    Ran uv run black . --check — passes cleanly with all files compliant.

  3. Core BrowserGym Tests:
    Verified core Playwright action tests pass without regressions.

Security & Edge Cases Considered

  • Empty vs Unset Environment Variables: Handled both None and "" values.
  • Backward Compatibility: Non-breaking change; existing callers that do not inspect the return value continue unaffected.
  • Fail-closed opt-in: Callers can still explicitly pass skip_if_not_set=False to abort with RuntimeError if the reset URL is not configured.
  • Inheritance: Verified VisualWebArenaInstance inherits full_reset() while honoring its VWA_FULL_RESET variable.

Mentions

cc @Surya-5555 (maintainer) and @nikolas-sapa (issue author) — thank you both for the helpful discussion and feedback on the issue!

…ixes ServiceNow#406)

Signed-off-by: tharun_varshan_s <tharunvarshan.s087@gmail.com>

This branch has not been deployed

No deployments
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.

full_reset() gives no signal whether the environment was reset; skipped reset is indistinguishable from a successful one

1 participant