Skip to content

Consolidate WebArena environments and add isolated Rainbond pools - #18

Merged
lanyue-llk merged 23 commits into
mainfrom
codex/webarena-rainbond-isolated-pool
Sep 17, 2026
Merged

lanyue-llk merged 23 commits into
mainfrom
codex/webarena-rainbond-isolated-pool

Conversation

@lanyue-llk

@lanyue-llk lanyue-llk commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

WebArena 的 Docker 运行工具和 Rainbond 槽位管理统一由本仓库维护,部署与评测使用同一份镜像锁。并发执行者各自独占完整的有状态槽位,只读 Wikipedia 可共享。

主要改动:

  • 将 webarena-env 的 Docker 管理、归档校验和部署契约纳入 harness/environments/webarena/ 与 deploy/webarena/,共用 Docker backend 和站点初始化实现,保留 MIT 许可证。
  • 提供 Rainbond 创建、扩缩容、租约、重置和删除流程;回执与运行配置原子更新,失败清理保留原始异常。释放槽位时使用槽位锁,重建不会阻塞其他空闲槽位的分配。
  • 并发需求和空闲预留量决定槽位数;不固定两倍资源,预留量可为零。资源余量不足时可选择先停止旧槽位再重建,失败槽位保持不可分配。
  • 释放操作记录自己的重置起点,不将任务准备阶段的实例替换误认作清理完成;配置写入和租约删除失败后可安全重试。
  • 评测器重置后重新生成网站 URL,资格运行只把不同的槽位分给并发批次。镜像缺失、digest 不一致或回执不匹配时拒绝使用。

验证:make ci 通过;环境管理、CLI 与公开 WebArena 回归测试 418 项通过、3 项跳过。仓库内镜像/端口契约检查覆盖全部运行与部署模板,包括相对路径解析。2026-09-17 对两个四站点槽位和共享 Wikipedia 发送 72 个并发 HTTP 请求,全部返回预期页面,最慢站点 P95 为 2.37 秒。

当前部署含两个可用槽位、一个共享 Wikipedia,以及一个已停止但仍用于复制和重置的基线应用。Map 未部署。HTTP 测试覆盖连通性和基本渲染,不代替登录、数据库隔离或正式评测初态验证。共享 Kiwix amd64 镜像身份与官方 Wikipedia arm64 镜像不同,不能复用官方运行身份回执。

webarena-verified 保持独立的任务与评分器依赖;其修正由 fork PR #1 审查,本 PR 不更改既有评测分数。

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 10s —— View job


PR Review: Add isolated Rainbond WebArena warm pool

  • Gather context
  • Read changed files and diff
  • Identify bugs and security issues
  • Post findings

Two bugs found. No security issues.

Bug 1: provision_pool cleanup swallows the original exception and leaks slices

harness/trajectory_replay/public_webarena/rainbond_slices.py lines 713–716

except BaseException:
    for slot_dir in reversed(created):
        pool.destroy(slot_dir / "slice-receipt.json")
    raise

If pool.destroy() raises (e.g. a network error during cleanup), the raise at the end never executes — the cleanup exception propagates instead, losing the original failure. Any remaining slices after the failed destroy call are also never cleaned up (Rainbond resource leak). Each destroy() call should be individually wrapped to allow cleanup to continue regardless: Fix this →

Bug 2: acquire_slice uses lexicographic sort, so slice-10 is acquired before slice-2

harness/trajectory_replay/public_webarena/rainbond_slices.py line 817

for slot_dir in sorted(output_dir.glob("slice-*")):

sorted() on Path objects is lexicographic. With ten or more slices, slice-10 sorts before slice-2. All slices are functionally equivalent so this causes no data corruption, but it makes acquisition order non-intuitive and may complicate debugging. Fix with a numeric key: Fix this →


@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 57s —— View job


PR #18 Code Review

Review 结论

有条件通过

整体结构合理:隔离设计、原子租约、蓝绿替换逻辑清晰,测试覆盖主路径,凭据隔离(仅读环境变量,receipt 不含 token)符合 AGENTS.md 要求。发现 3 处 P1 和 3 处 P2 问题。


问题及建议

P1 — 合并前建议修

  • [P1] harness/trajectory_replay/public_webarena/rainbond_slices.py:651-657:reset() 中,旧 app DELETE 失败后进入 except BaseException 清理分支,若清理时第二次 api.request("DELETE", replacement_app) 也抛异常,raise 会抛出该清理异常而丢失原始异常,系统可能同时持有旧 app 和新 app 却无法判断起因。建议将清理 DELETE 包在独立的 try/except 中再 raise 原始异常:

    except BaseException as exc:
        try:
            self.api.request("DELETE", f"{self._app_path(int(replacement['app_id']))}?force=1")
            replacement_path.unlink(missing_ok=True)
        except Exception:
            pass
        raise exc

    Fix this →

  • [P1] harness/trajectory_replay/public_webarena/rainbond_slices.py:369-370:_configure_component() 中,若 Rainbond API 响应缺少 image 字段(actual_image 为空字符串),pinned image 校验被静默跳过,无法检测到镜像漂移。建议去掉 if actual_image and 前置条件,改为当 actual_image 为空时直接用 expected_image(已赋值给 runtime_image),或显式抛出异常:

    if not actual_image:
        raise RuntimeError(f"Rainbond copied {site} component has no image in response")
    if _image_digest(actual_image) != _image_digest(expected_image):
        raise RuntimeError(...)

    Fix this →

  • [P1] harness/trajectory_replay/public_webarena/rainbond_slices.py:762-766:scale_pool() 的 ThreadPoolExecutor 中,若某个 create_slot future 抛异常,future.result() 在 for future in as_completed(futures) 中立即传播,此时其他已完成的 future 对应的 Rainbond app 已创建但未加入 existing,也无任何清理,造成资源泄漏。provision_pool() 有 except BaseException 清理块,scale_pool() 缺乏同等保护。建议在 ThreadPoolExecutor 外层添加 except,对已成功创建但未完成注册的 slot 执行 destroy()。
    Fix this →


P2 — 可后续优化

  • [P2] harness/trajectory_replay/public_webarena/rainbond_slices.py:617-619:provision() 的 except BaseException 清理块中,若 api.request("DELETE", app_id) 本身抛异常,原始异常被覆盖。同上,建议 try/except 包裹清理调用后 raise 原始异常。

  • [P2] harness/trajectory_replay/public_webarena/rainbond_slices.py:711-716:provision_pool() 的清理循环(for slot_dir in reversed(created): pool.destroy(...))缺少 try/except,首次 destroy() 失败即中止后续清理,剩余 slot 泄漏。建议循环内捕获异常继续执行,最后统一抛出。

  • [P2] scripts/run_webarena_qualification.py:428:zip(wave, config_paths, strict=False) 后立即检查 len(assigned) != len(wave),与 strict=False 语义重复。直接用 strict=True 更简洁(或保留现有方式并加注释说明为何需要不等长截断)。


其他观察(不阻塞合并)

  • runner.py 的改动(line 951 移除 materialize_task_actions 前置调用、在 retry loop 内 line 1005 重新调用)逻辑正确:对 Rainbond 环境 reset 后 URL 变更的场景,per-retry 调用保证了 URL 最新;webarena_verified 块内始终调用(含 warm reuse 分支),无遗漏。
  • _pool_lock() 使用 fcntl.flock(),仅对跨进程有效,同进程多线程不受保护。当前 scale_pool() 在锁内创建 ThreadPoolExecutor 是正确的(并行 provision 全在锁内完成),但若调用方在多线程环境中并发调用 scale_pool(),锁不生效。当前 CLI 用法下不构成问题。
  • rainbond_slices.py:677:runtime_config() 将 image_lock 的绝对路径写入 config,仅在同机器间传递时有效,文档中无说明。

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35067897956. Please check Claude's inline feedback and summary comment for details.

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 49s —— View job


PR Review: Add isolated Rainbond WebArena warm pool

  • Gather context
  • Read changed files and diff
  • Identify bugs and security issues
  • Post findings

The bugs flagged in the previous two reviews are still present in the current code. One additional issue found.

Still-unfixed P1 bugs

reset() cleanup swallows the original exception (rainbond_slices.py:704–711)

except BaseException:
    self.api.request("DELETE", f"{self._app_path(int(replacement['app_id']))}?force=1")
    replacement_path.unlink(missing_ok=True)
    if stopped_previous:
        self._restore_previous_app(previous)
    raise  # never reached if any line above throws

If the cleanup DELETE or _restore_previous_app raises, raise is never reached and the original exception is lost. Wrap each cleanup step in its own try/except and re-raise the original. Fix this →

scale_pool() leaks Rainbond apps when a future fails (rainbond_slices.py:816–820)

for future in as_completed(futures):
    slot, slot_dir = future.result()   # raises; loop exits immediately
    existing[slot] = slot_dir          # completed-but-unregistered futures are never destroyed

When future.result() raises, the loop exits. Other already-completed futures' apps are created in Rainbond but their slot_dirs were never appended to existing, so they are never cleaned up. Add an except BaseException block (analogous to provision_pool) that destroys any successfully registered slots. Fix this →


Additional bug (not previously flagged)

_configure_initializer silently skips image pinning validation (rainbond_slices.py:442)

actual_image = str(initializer.get("image") or "")
if actual_image and _image_digest(actual_image) != _image_digest(self.config.initializer_image):
    raise RuntimeError("Rainbond copied initializer image mismatch")

Same pattern as the _configure_component issue (P1 in the previous review): if the Rainbond copy API returns no image field, actual_image is empty, the condition is skipped, and an unpinned initializer image silently runs. Add an explicit if not actual_image: raise RuntimeError(...) guard before the digest comparison. Fix this →


No security issues found.

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

PR #18 Code Review

  • 获取上下文
  • 读取 AGENTS.md
  • 读取关键变更文件
  • 分析问题
  • 发布 review 结论

View job run

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35068621654. Please check Claude's inline feedback and summary comment for details.

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 51s —— View job


PR #18 Code Review

  • Gather context
  • Read CLAUDE.md
  • Read changed files
  • Analyze for bugs and security issues
  • Post findings

Two bugs found. No new security issues.

Bug 1: _configure_initializer silently skips image check when API returns no image

harness/trajectory_replay/public_webarena/rainbond_slices.py lines 457–460

if actual_image and _image_digest(actual_image) != _image_digest(self.config.initializer_image):
    raise RuntimeError("Rainbond copied initializer image mismatch")

_configure_component was fixed to raise when actual_image is empty, but _configure_initializer still uses the old if actual_image and ... guard. When Rainbond returns no image for the initializer component, the pinned-image check is silently bypassed and image drift goes undetected. Apply the same fix: Fix this →

Bug 2: scale_pool excess-slot destroy loop has no error handling

harness/trajectory_replay/public_webarena/rainbond_slices.py lines 911–920

for slot in excess_slots:
    slot_dir = existing.pop(slot)
    pool.destroy(slot_dir / "slice-receipt.json")   # no try/except
    ...

Every other destroy call in this file is wrapped with _attempt_cleanup. Here, if the first pool.destroy() raises (e.g. a Rainbond network error), the loop aborts immediately and all remaining excess slots are leaked — Rainbond apps keep running and consuming resources indefinitely. Wrap each call with _attempt_cleanup (or equivalent) and re-raise after iterating all slots: Fix this →

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 51s —— View job


PR #18 Code Review

  • 获取上下文
  • 读取 AGENTS.md
  • 读取关键变更文件
  • 分析问题
  • 发布 review 结论

Review 结论

有条件通过

整体结构可靠:_attempt_cleanup 工具函数统一了所有清理路径的异常处理,provision_pool/scale_pool/reset 的原始异常覆盖问题已修复;_configure_component 的空 image 静默跳过已修复;acquire_slice 的词典序排序已修复;凭据隔离正确(token_env 存储变量名、receipt 不含 token、.env.example 符合 AGENTS.md 要求)。发现 1 处 P1 和 2 处 P2。


问题及建议

[P1] rainbond_slices.py:457–460:_configure_initializer() 中初始化器镜像校验仍使用 if actual_image and ...,API 响应缺少 image 字段时静默跳过,与 _configure_component() 第 381 行已修复的相同问题不一致。initializer 负责 WebArena 全环境状态写入,跳过镜像校验会在镜像漂移时无法检测。建议改为显式 raise,与 _configure_component 对齐:

if not actual_image:
    raise RuntimeError("Rainbond copied initializer has no image in response")
if _image_digest(actual_image) != _image_digest(self.config.initializer_image):
    raise RuntimeError("Rainbond copied initializer image mismatch")

Fix this →

[P2] webarena_environment.py:463–466:_reset_impl() 调用 self.rainbond_pool.reset() 后更新内存中 self.environments,但不写 webarena.config.json。若 shard 进程在 task 间崩溃重启,WebArenaEnvironment.__init__ 从磁盘读取过时 URLs,而 receipt 已含新 app 的 URLs,verify_runtime() 的 URL 一致性校验(webarena_environment.py:252–254)将立即失败,shard 无法自动恢复,需人工干预。建议 _reset_impl() 在 Rainbond reset 后同步调用 _atomic_json(self.config_path, ...) 更新配置文件,或在 verify_runtime() 中对 Rainbond 模式优先以 receipt 为权威 URL 来源。

[P2] rainbond_slices.py:390:"runtime_image": actual_image or expected_image 中 or expected_image 是死代码——第 381 行已对 not actual_image 抛出异常,执行到此处时 actual_image 必然非空。不影响正确性,但有误导性,建议直接写 "runtime_image": actual_image。


其他观察(不阻塞合并)

  • 前两次 review 指出的所有 P1/P2(_attempt_cleanup 覆盖缺失、provision_pool 清理循环、scale_pool future 泄漏、_configure_component 空 image、acquire_slice 排序)已在当前代码中全部修复,独立验证一致。
  • runtime_config() line 769 将绝对路径 image_lock 写入 config,仅在同机器上可用,文档中已有说明,当前部署模型下可接受。
  • scripts/run_webarena_qualification.py:432:zip(wave, config_paths, strict=False) 后检查 len(assigned) != len(wave) 是故意截断(允许多余 config),注释已说明,行为正确。
  • _pool_lock 使用 fcntl.flock 对同进程多线程无效,但 CLI 调用路径下 scale_pool 在锁内统一完成并行 provision,无实际问题。

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35069031850. Please check Claude's inline feedback and summary comment for details.

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 2m 55s —— View job


PR #18 Code Review

  • 获取上下文
  • 读取 AGENTS.md
  • 读取关键变更文件
  • 独立分析当前代码状态
  • 发布 review 结论

Review 结论

有条件通过

前序各轮 review 指出的所有 P1/P2(_attempt_cleanup 覆盖缺失、provision_pool/scale_pool future 泄漏、scale_pool excess-slot 循环无保护、_configure_component 和 _configure_initializer 空 image 静默跳过、acquire_slice 词典序排序)已在当前代码中全部修复,独立验证一致。runner.py 的 materialize_task_actions 移位逻辑正确:处于 webarena_verified 块内、assert webarena_environment is not None 之后,warm reuse 路径下使用内存中最新 self.environments,行为符合预期。


问题及建议

[P1] webarena_environment.py:463–466:_reset_impl() 调用 self.rainbond_pool.reset() 后只更新内存中的 self.environments,不写入 webarena.config.json。而 rainbond_slices.py:provision() 已原子写入 receipt 文件。若 shard 进程在任一 task reset 后崩溃并重启,新进程从磁盘加载旧 webarena.config.json(含旧 app URLs),verify_runtime() 第 252–253 行:if configured_urls != [site_url]: raise RuntimeError(...) 必然失败,切片无法自动恢复。建议在 _reset_impl() 更新 self.environments 后同步落盘,复用 release_slice() 的模式:

from .rainbond_slices import _atomic_json
# ...
receipt = self.rainbond_pool.reset(self.rainbond_receipt_path)
for site, component in receipt["components"].items():
    token = f"__{site.upper()}__"
    self.environments[token] = {"urls": [component["site_url"]]}
new_runtime = self.rainbond_pool.runtime_config(receipt)
_atomic_json(self.config_path, {**self.config, "environments": new_runtime["environments"]})

Fix this →

[P2] rainbond_slices.py:390:"runtime_image": actual_image or expected_image 中 or expected_image 是死代码——第 380–381 行已对 not actual_image 抛出异常,执行到此处时 actual_image 必然非空。建议改为 "runtime_image": actual_image。


其他观察(不阻塞合并)

  • _pool_lock() 使用 fcntl.flock,对同进程多线程无效;但 scale_pool 已在锁内统一完成并行 provision,CLI 路径下无实际竞争问题。
  • runtime_config() line 769 将绝对路径 image_lock 写入 config,仅在同机器可用;文档中已有说明,当前部署模型下可接受。

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 55s —— View job


PR #18 Review

  • Gather context
  • Read changed files
  • Identify bugs and security issues
  • Post findings

All previously flagged P1 bugs are fixed in this revision. One P2 remains.

P2: _reset_impl does not persist updated URLs to webarena.config.json

harness/trajectory_replay/public_webarena/webarena_environment.py lines 463–466

receipt = self.rainbond_pool.reset(self.rainbond_receipt_path)
for site, component in receipt["components"].items():
    token = f"__{site.upper()}__"
    self.environments[token] = {"urls": [component["site_url"]]}

self.environments is updated in memory with the new app's URLs, but webarena.config.json on disk is not updated. After reset(), slice-receipt.json on disk is atomically replaced by reset() itself with the new receipt (new app_id and new site_url values). If the shard process crashes and restarts between tasks, WebArenaEnvironment.__init__ reloads self.environments from the stale disk config, then verify_runtime() at line 253 compares those stale URLs against the fresh receipt and raises RuntimeError: Rainbond slice URL mismatch for <site> — the shard cannot recover without manual intervention.

release_slice() (line 1006) correctly calls _atomic_json(slot_dir / "webarena.config.json", pool.runtime_config(receipt)) after reset. The same call should be added to _reset_impl(). Fix this →


No security issues found.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35069787517. Please check Claude's inline feedback and summary comment for details.

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 5m 3s —— View job


PR #18 Code Review

  • 获取上下文
  • 读取 AGENTS.md
  • 读取关键变更文件
  • 独立分析当前代码状态
  • 发布 review 结论

Review 结论

有条件通过

前序各轮 review 指出的所有 P1/P2 均已在当前代码中修复,独立验证通过:

  • _attempt_cleanup 工具函数统一了全部清理路径(provision()、reset()、provision_pool()、scale_pool() 并行 future 和 excess-slot 循环),原始异常不再被清理异常覆盖。
  • _configure_component()(line 380)和 _configure_initializer()(line 457)均已对空 actual_image 显式 raise,镜像校验无法静默跳过。
  • acquire_slice()(line 976)已改用数值排序键,slice-10 不再排在 slice-2 之前。
  • _reset_impl()(line 468)改为调用 persist_runtime_config(),原子写入 webarena.config.json,崩溃重启后 URL 与 receipt 保持一致;test_webarena_environment.py:246 验证了磁盘落盘行为。
  • _configure_component() 返回值死代码 or expected_image 已移除,直接写 actual_image(line 391)。
  • run_webarena_qualification.py:432 的 zip(strict=False) + 长度检查属于有意截断设计(允许多余切片),逻辑正确。

问题及建议

[P2] scripts/manage_webarena_rainbond_pool.py:92:CLI reset 命令在 pool.reset() 成功后用 target.write_text(...) 写 webarena.config.json,而 pool.reset() 已原子替换了 receipt。若进程在两步之间崩溃,config 文件与 receipt 不一致。运行时路径不受影响(WebArenaEnvironment.__init__ 始终从 receipt 重新派生 self.environments),但与其他路径一律使用 _atomic_json() 的惯例不一致。建议替换为 pool.persist_runtime_config(receipt, target) 或导入 _atomic_json 后写入:

from harness.trajectory_replay.public_webarena.rainbond_slices import _atomic_json
# ...
receipt = pool.reset(slot_dir / "slice-receipt.json")
target = slot_dir / "webarena.config.json"
_atomic_json(target, pool.runtime_config(receipt))
print(target)

其他观察(不阻塞合并)

  • runner.py:1005 materialize_task_actions(base_task) 在 webarena_verified 块内无条件调用,含 warm reuse 路径,行为正确:每次 attempt 均以最新 self.environments(reset 后已更新)重新解析 URL。
  • runtime_config() line 769 将绝对路径 image_lock 写入 config,仅在同机器间有效;pool_config 同理;文档已说明,当前部署模型下可接受。
  • _pool_lock() 使用 fcntl.flock,对同进程多线程无效;但 scale_pool 在锁内统一完成并行 provision,CLI 路径无实际竞争。
  • qualification_waves 的 isolated 逻辑:isolated_instances > 1 时忽略 site 冲突检查,仅按 min(max_workers, isolated_instances) 限制每波并发数;assigned 分配逻辑(zip(wave, config_paths, strict=False) + 长度断言)确保切片不被超额分配,行为正确。

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 5s —— View job


PR #18 Code Review

  • Gather context
  • Read changed files
  • Identify bugs and security issues
  • Post findings

All bugs flagged across previous review rounds are fixed in the current code. No new issues found.

Confirmed fixed:

  • reset() cleanup — now uses _attempt_cleanup for each step; original exception is preserved
  • provision_pool cleanup loop — _attempt_cleanup wraps each destroy(), errors accumulated and cleanup continues
  • scale_pool future leak — first_error pattern collects all future results before raising; rollback destroys only successfully-created slots
  • scale_pool excess-slot destroy loop — first_destroy_error accumulation lets the loop continue through failures before re-raising
  • _configure_component empty image — now raises RuntimeError explicitly instead of silently skipping
  • _configure_initializer empty image — fixed to match _configure_component; raises explicitly
  • _configure_component dead code — "runtime_image": actual_image or expected_image simplified to actual_image (line 390)
  • acquire_slice lexicographic sort — replaced with slot_sort_key (numeric extraction)
  • _reset_impl disk persistence — now calls persist_runtime_config which atomically writes updated URLs to webarena.config.json; crash-restart recovery path is safe
  • _pool_lock in-process threading — now combines threading.Lock (per resolved path) with fcntl.flock for both in-process and cross-process exclusion

No security issues found.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35070378865. Please check Claude's inline feedback and summary comment for details.

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 2m 16s —— View job


PR #18 Code Review

  • Gather context
  • Read CLAUDE.md and changed files
  • Analyze current code state
  • Post findings

All previously flagged bugs are confirmed fixed in the current code. No new issues found.

Confirmed fixed:

  • reset() cleanup — _attempt_cleanup wraps each step; original exception preserved (rainbond_slices.py:721–742)
  • provision_pool cleanup loop — first_error pattern collects all futures before raising; _attempt_cleanup wraps each rollback destroy (rainbond_slices.py:808–827)
  • scale_pool future leak — first_error pattern; rollback destroys only successfully created slots (rainbond_slices.py:887–904)
  • scale_pool excess-slot destroy loop — first_destroy_error accumulation allows loop to continue through failures before re-raising (rainbond_slices.py:917–938)
  • _configure_component empty image — raises RuntimeError explicitly (rainbond_slices.py:380–381)
  • _configure_initializer empty image — fixed to match; raises explicitly (rainbond_slices.py:457–458)
  • _configure_component dead code — "runtime_image": actual_image or expected_image simplified to actual_image (rainbond_slices.py:390)
  • acquire_slice lexicographic sort — replaced with slot_sort_key extracting integer suffix (rainbond_slices.py:970–974)
  • _reset_impl disk persistence — calls persist_runtime_config which atomically writes updated URLs to webarena.config.json (webarena_environment.py:468–472)
  • manage_webarena_rainbond_pool.py reset — uses pool.persist_runtime_config(receipt, target) (manage_webarena_rainbond_pool.py:91)
  • _pool_lock in-process threading — combines threading.Lock per resolved path with fcntl.flock for both in-process and cross-process exclusion (rainbond_slices.py:832–844)

No security issues found.

@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 5m 4s —— View job


PR #18 Code Review

  • 读取 AGENTS.md
  • 读取关键变更文件
  • 独立分析当前代码状态
  • 发布 review 结论

Review 结论

通过

对当前代码独立逐行核查,前序各轮 review 报告的全部 P1/P2 均已修复,独立验证结论一致:

  • _attempt_cleanup 统一覆盖了 provision()、reset()、provision_pool()、scale_pool() 并行 future、excess-slot 循环及 destroy CLI 批量循环的所有清理路径,原始异常不再被覆盖。
  • _configure_component()(line 380-381)和 _configure_initializer()(line 457-458)均已对空 actual_image 显式 raise RuntimeError,镜像校验无法静默跳过。
  • _configure_component() 死代码 or expected_image 已清除(line 390)。
  • acquire_slice() 已改用数值排序键 slot_sort_key(line 970-974)。
  • _reset_impl() 调用 persist_runtime_config()(line 468-472),原子写入 webarena.config.json;test_webarena_environment.py:246-248 验证了磁盘落盘行为。
  • manage_webarena_rainbond_pool.py:91 reset 命令已改用 pool.persist_runtime_config(receipt, target)。
  • _pool_lock() 同时持有 threading.Lock(进程内)和 fcntl.flock(跨进程)。
  • 凭据隔离正确:token_env 仅存环境变量名,receipt 不含 token,.env.example 只列变量名,符合 AGENTS.md 要求。

问题及建议

未发现需要阻塞合并的问题。残余风险:

  • _coalesce_internal_ports(rainbond_slices.py:422-441):删除并重建 control port 后立即调用 inner_ports() 验证,没有重试逻辑。若 Rainbond 异步更新 k8s service name 存在延迟,可能偶发误报 RuntimeError。当前测试覆盖了正常路径;生产环境中如果遇到偶发失败需关注此处。
  • runtime_config()(line 762-768)将 image_lock 和 pool_config 写入 config 时均使用绝对路径,仅在同机器间有效;文档中已有说明,当前部署模型下可接受。
  • reset_strategy = "stop_before_replace" 时,若 provision() 失败,旧 app 已停止但不会自动重启,shard 需人工干预;这是已知设计权衡,默认策略 replace_before_destroy 不受此影响。

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35101632947. Please check Claude's inline feedback and summary comment for details.

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 5m 51s —— View job


PR #18 Code Review

  • 获取上下文
  • 读取 AGENTS.md
  • 读取关键变更文件
  • 独立分析当前代码状态
  • 发布 review 结论

Review 结论

通过

对当前代码逐文件独立核查,前序各轮 review 报告的全部 P1/P2 均已修复,与最近两轮中文 review 的结论一致:

  • _attempt_cleanup 覆盖所有清理路径(provision()、reset()、provision_pool()、scale_pool() 并行 future 及 excess-slot 循环)。
  • _configure_component() line 380–381、_configure_initializer() line 463–464 对空 actual_image 均显式 raise RuntimeError,镜像校验不可静默跳过。
  • acquire_slice() 使用 slot_sort_key 数值排序(line 999–1003)。
  • _reset_impl() 调用 persist_runtime_config()(line 468–472)原子写入 webarena.config.json;test_webarena_environment.py:246–248 验证磁盘落盘行为。
  • manage_webarena_rainbond_pool.py:91 reset 命令已改用 pool.persist_runtime_config(receipt, target)。
  • _pool_lock() 同时持有 threading.Lock(进程内)和 fcntl.flock(跨进程)。
  • "runtime_image": actual_image or expected_image 死代码已清除(line 390)。
  • 凭据隔离正确:token_env 仅存变量名,receipt 不含 token,.env.example 符合 AGENTS.md 要求。

问题及建议

未发现需要阻塞合并的问题。残余风险:

  • [P2] rainbond_slices.py:1042:release_slice() 在 pool.reset() 和 _atomic_json 均成功后调用 lease_path.unlink(),若 unlink() 抛异常(文件系统故障),新 app 已在运行但 lease 仍引用旧 app_id,该 slice 对 acquire_slice 不可见,需人工清理 lease 文件才能恢复。其余路径(provision_pool、scale_pool)均有对应的异常处理模式;此处可改用 _attempt_cleanup 或 try/except,但在 Rainbond 部署实践中文件系统 unlink 失败极少见,不阻塞合并。

  • _coalesce_internal_ports(line 438–448):DELETE/POST 后重试最多 3 次验证 k8s_service_name 是否与 site_host 一致,无退避重启,若 Rainbond 异步刷新延迟较长可能偶发误报 RuntimeError。已文档化,当前 smoke test 通过,生产观察中需关注。

  • runtime_config() 写入绝对路径 image_lock/pool_config,仅同机器有效;文档中已说明,当前部署模型下可接受。

  • stop_before_replace 策略下若 provision() 失败,旧 app 已停止但不会自动重启;已知设计权衡,默认策略 replace_before_destroy 不受影响。

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 4m 43s —— View job


PR #18 Code Review

  • Gather context
  • Read changed files
  • Identify bugs and security issues
  • Post findings

All previously flagged bugs are confirmed fixed. No new issues found in the latest commit.

Confirmed fixed (latest commit 1db1161):

  • _delete_app added — stops all services and waits for closed/undeploy before calling DELETE, so Rainbond does not see live containers removed at the control-plane level. All existing delete call sites updated to use it.
  • _coalesce_internal_ports — retry loop (3 attempts with back-off) tolerates Rainbond read-model lag after the control port is recreated. Writes are not repeated on retry.
  • All cleanup paths (provision, reset, provision_pool, scale_pool futures and excess-slot loop) use _attempt_cleanup; original exceptions are preserved.
  • _configure_component and _configure_initializer both raise explicitly on empty actual_image.
  • acquire_slice uses numeric sort key (slot_sort_key).
  • _reset_impl calls persist_runtime_config, writing updated URLs to webarena.config.json atomically.
  • manage_webarena_rainbond_pool.py reset uses pool.persist_runtime_config.
  • _pool_lock holds both threading.Lock and fcntl.flock.

No security issues found.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35177165169. Please check Claude's inline feedback and summary comment for details.

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 1m 30s —— View job


PR #18 Code Review

  • Gather context
  • Read changed files and diff
  • Identify bugs and security issues
  • Post findings

All previously flagged bugs are confirmed fixed in the current code. No new issues found.

Confirmed fixed:

  • All cleanup paths (provision, reset, provision_pool, scale_pool futures and excess-slot loop) use _attempt_cleanup; original exceptions preserved
  • _configure_component and _configure_initializer both raise explicitly on empty actual_image (lines 380–381, 463–464)
  • acquire_slice uses numeric sort key slot_sort_key (lines 999–1001)
  • _reset_impl calls persist_runtime_config, atomically writing updated URLs to webarena.config.json (lines 468–472)
  • manage_webarena_rainbond_pool.py reset uses pool.persist_runtime_config(receipt, target) (line 91)
  • _pool_lock combines threading.Lock (per resolved path) with fcntl.flock for both in-process and cross-process exclusion
  • "runtime_image": actual_image or expected_image dead code removed; now just actual_image (line 390)

No security issues found.

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 30s —— View job


PR #18 Code Review

  • 获取上下文
  • 读取 AGENTS.md
  • 读取关键变更文件
  • 独立分析当前代码状态
  • 发布 review 结论

Review 结论

通过

对当前代码逐文件独立核查,前序各轮 review 报告的全部 P1/P2 均已修复,独立验证结论一致:

  • _attempt_cleanup 统一覆盖了 provision()、reset()、provision_pool()、scale_pool() 并行 future 及 excess-slot 循环所有清理路径,原始异常不再被清理异常覆盖。
  • _configure_component()(line 380–381)和 _configure_initializer()(line 463–464)均已对空 actual_image 显式 raise RuntimeError,镜像校验不可静默跳过。
  • acquire_slice() 使用 slot_sort_key 数值排序(line 999–1001),slice-10 不再排在 slice-2 之前。
  • _reset_impl() 调用 persist_runtime_config()(line 468–472)原子写入 webarena.config.json;test_webarena_environment.py:246–248 验证了磁盘落盘行为;verify_runtime() line 257 的 URL 一致性校验在重启后可正确通过。
  • manage_webarena_rainbond_pool.py:91 reset 命令已改用 pool.persist_runtime_config(receipt, target)。
  • _pool_lock() 同时持有 threading.Lock(进程内)和 fcntl.flock(跨进程)。
  • "runtime_image": actual_image or expected_image 死代码已清除,直接写 actual_image(line 390)。
  • release_slice() 在 unlink() 失败后可幂等重试:current_receipt["app_id"] != lease["app_id"] 时核查 runtime_config_path 是否已与新 receipt 匹配,匹配则直接解锁;test_release_retry_only_unlinks_after_reset_was_persisted 覆盖该路径。
  • 凭据隔离正确:token_env 仅存变量名,receipt 不含 token,.env.example 符合 AGENTS.md 要求。

问题及建议

未发现需要阻塞合并的问题。残余风险:

  • [P2] rainbond_slices.py:857:provision_pool() 返回值 return sorted(created) 使用 Path 字典序排序,当切片数 > 9 时 slice-10 排在 slice-2 之前。仅影响 CLI 输出顺序(无功能影响)。对比 scale_pool() line 969 [existing[slot] for slot in sorted(existing)] 已按整数键排序,两者行为不一致。建议改为 return sorted(created, key=lambda p: (int(p.name.removeprefix("slice-")) if p.name.removeprefix("slice-").isdigit() else float("inf")))。
  • destroy() line 729–730:_delete_app() 成功后 receipt_path.unlink() 若抛异常(文件系统故障),receipt 保留但 Rainbond app 已删除,下次重试时 _delete_app() 会对不存在的 app 发出 DELETE 请求(Rainbond 404 → RuntimeError)。极低概率的文件系统级故障,不阻塞合并。
  • _coalesce_internal_ports(line 438–444)重试验证最多 3 次(睡眠 1s/2s),写操作不重试。若 Rainbond 读模型刷新延迟超过 3s 会误报 RuntimeError,已有注释说明设计意图,可在生产中观察。
  • runtime_config() 将 image_lock 和 pool_config 绝对路径写入 config,仅同机器间有效;文档已说明,当前部署模型下可接受。

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35177849766. Please check Claude's inline feedback and summary comment for details.

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 40s —— View job


PR #18 Code Review

  • 获取上下文
  • 读取 AGENTS.md
  • 读取关键变更文件
  • 独立分析当前代码状态
  • 发布 review 结论

Review 结论

通过

对当前代码逐文件独立核查,前序各轮 review 报告的全部 P1/P2 均已修复,最新两个 commit(75b8fc2、4707864)在此基础上新增了两项改进:

  • _slot_sort_key 提升为模块级函数,provision_pool 返回值(line 867)与 acquire_slice(line 1009)统一使用,词典序问题彻底消除。
  • _delete_app 对 Rainbond 404 响应提前返回(line 636–640),destroy() 重试时不再因 app 已删除而误报。
  • release_slice 新增幂等重试路径(line 1049–1062):若 lease_path.unlink() 失败,重试时检测到 current_receipt["app_id"] != lease["app_id"],确认 runtime config 已落盘后直接解锁,不重复 pool.reset();test_release_retry_only_unlinks_after_reset_was_persisted 覆盖该路径。

已独立确认的其余修复点(与上两轮 review 一致):

  • _attempt_cleanup 统一覆盖所有清理路径,原始异常不被覆盖。
  • _configure_component(line 385–386)和 _configure_initializer(line 468–469)均对空 actual_image 显式 raise。
  • _reset_impl() 调用 persist_runtime_config(base_config=self.config) 原子落盘。
  • manage_webarena_rainbond_pool.py:91 使用 pool.persist_runtime_config(receipt, target)。
  • _pool_lock() 同时持有 threading.Lock 和 fcntl.flock。
  • "runtime_image": actual_image or expected_image 死代码已清除(line 395)。
  • 凭据隔离正确:token_env 仅存变量名,receipt 不含 token,.env.example 符合 AGENTS.md 要求。

问题及建议

未发现需要阻塞合并的问题。残余风险:

  • _coalesce_internal_ports(line 443–449):3 次重试间隔 1s/2s,若 Rainbond 读模型刷新延迟 > 3s 会误报 RuntimeError;写操作不重试,设计意图已注释,生产中需关注偶发失败。
  • release_slice 在持有 _pool_lock 期间调用 pool.reset()(可能耗时数分钟),期间同一 output_dir 的所有 acquire/release 操作均被阻塞;属已知设计权衡,不影响正确性。
  • runtime_config() 将 image_lock 和 pool_config 绝对路径写入 config,仅同机器间有效;文档已说明,当前部署模型下可接受。
  • stop_before_replace 策略下若 provision() 失败,旧 app 已停止但不会自动重启;默认策略 replace_before_destroy 不受此影响。

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 53s —— View job


PR #18 Code Review

  • Gather context
  • Read changed files
  • Identify bugs and security issues
  • Post findings

All previously flagged bugs are confirmed fixed in the current code. No new issues found.

Confirmed fixed:

  • All cleanup paths (provision, reset, provision_pool, scale_pool futures and excess-slot loop) use _attempt_cleanup; original exceptions preserved
  • _configure_component and _configure_initializer both raise explicitly on empty actual_image (lines 385–386, 468–469)
  • acquire_slice uses numeric sort key _slot_sort_key (line 1009)
  • _reset_impl calls persist_runtime_config, atomically writing updated URLs to webarena.config.json (lines 468–472)
  • manage_webarena_rainbond_pool.py reset uses pool.persist_runtime_config(receipt, target) (line 91)
  • _pool_lock combines threading.Lock (per resolved path) with fcntl.flock for both in-process and cross-process exclusion
  • provision_pool return value uses _slot_sort_key for numeric ordering (line 867)
  • _delete_app stops all services and waits for closed/undeploy before issuing DELETE, with 404-on-GET handled gracefully for idempotent retries

No security issues found.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35179448046. Please check Claude's inline feedback and summary comment for details.

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 7m 19s —— View job


PR #18 Code Review

  • Gather context
  • Read changed files and diff
  • Identify bugs and security issues
  • Post findings

All previously flagged bugs are confirmed fixed in the current code. No new issues found.

Confirmed fixed in current code:

  • All cleanup paths (provision, reset, provision_pool, scale_pool futures and excess-slot loop) use _attempt_cleanup; original exceptions preserved (rainbond.py)
  • _configure_component and _configure_initializer both raise explicitly on empty actual_image (lines 385–386, 468–469)
  • acquire_slice uses _slot_sort_key for numeric sort (line 1005)
  • _reset_impl calls persist_runtime_config, atomically writing updated URLs to webarena.config.json
  • manage_webarena_rainbond_pool.py reset uses pool.persist_runtime_config(receipt, target) (line 91)
  • _pool_lock combines threading.Lock per resolved path with fcntl.flock for in-process and cross-process exclusion
  • provision_pool return value uses _slot_sort_key (line 864)
  • _delete_app 404-on-GET handled gracefully for idempotent retries (line 636–640)
  • release_slice idempotent retry path handles unlink() failure; test_release_retry_only_unlinks_after_reset_was_persisted covers this path
  • "runtime_image": actual_image or expected_image dead code removed (line 395)
  • Rainbond lifecycle consolidated into harness/environments/webarena/rainbond.py; webarena_environment.py imports from the new location

No security issues found.

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 4m 10s —— View job


PR #18 Code Review

  • 获取上下文
  • 读取 AGENTS.md
  • 读取关键变更文件
  • 独立分析当前代码状态
  • 发布 review 结论

Review 结论

通过

对当前代码(最新 commit 0c485e5 "Consolidate WebArena environment lifecycle in browser-eval")逐文件独立核查。前序各轮 review 报告的全部 P1/P2 均已修复,本次新增的 harness/environments/webarena/ 包(backend.py、cli.py、config.py、manager.py、map_archives.py、rainbond.py)是对 rainbond_slices.py 及其依赖逻辑的合理整合,无新增缺陷。

已独立确认的历次修复点:

  • _attempt_cleanup 覆盖 provision()、reset()、provision_pool()、scale_pool() 并行 future 及 excess-slot 循环所有清理路径。
  • _configure_component()(line 385–386)和 _configure_initializer()(line 468–469)均对空 actual_image 显式 raise,镜像校验不可静默跳过。
  • acquire_slice() 使用 _slot_sort_key 数值排序(line 1005),provision_pool() 返回值同用(line 864)。
  • _reset_impl() 调用 persist_runtime_config(base_config=self.config) 原子落盘(webarena_environment.py:364–369),测试 test_webarena_environment.py:246–248 验证了磁盘写入。
  • manage_webarena_rainbond_pool.py:91 reset 命令使用 pool.persist_runtime_config(receipt, target),测试 test_manage_webarena_rainbond_pool.py:28 验证了调用契约。
  • _pool_lock() 同时持有 threading.Lock(进程内)和 fcntl.flock(跨进程)。
  • "runtime_image": actual_image or expected_image 死代码已清除,直接写 actual_image(line 395)。
  • _delete_app() 对 404 响应提前返回(line 636–639),destroy 重试不再误报。
  • release_slice() 幂等重试路径(line 1050–1061)检测 app_id 变更后验证 config 已落盘,直接解锁,不重复 reset;test_release_retry_only_unlinks_after_reset_was_persisted 覆盖该路径。
  • 旧 harness/trajectory_replay/public_webarena/rainbond_slices.py 已完全移除,无孤立引用(grep 确认)。
  • 凭据隔离正确:token_env 仅存变量名,receipt 不含 token,.env.example 符合 AGENTS.md 要求。

问题及建议

未发现需要阻塞合并的问题。残余风险:

  • release_slice() line 1063 直接使用 _atomic_json(runtime_config_path, pool.runtime_config(receipt)),与 persist_runtime_config(base_config=None) 等价,但写法与其他路径略有差异;无功能影响。
  • 若 _atomic_json 本身失败(极罕见的文件系统故障),幂等重试路径会触发第二次 reset,泄漏第一次创建的 Rainbond app;与上一轮 review 结论一致,不阻塞合并。
  • _coalesce_internal_ports 重试间隔硬编码(1s/2s,最多 3 次),Rainbond 读模型延迟 > 3s 时可能偶发误报;已有注释说明,生产中需观察。
  • runtime_config() 将 image_lock 和 pool_config 绝对路径写入 config,仅同机器间有效;文档已说明,当前部署模型下可接受。
  • stop_before_replace 策略下若 provision() 失败,旧 app 已停止但不会自动重启;默认策略 replace_before_destroy 不受影响。

@lanyue-llk lanyue-llk changed the title Add isolated Rainbond WebArena warm pool Consolidate WebArena environments and add isolated Rainbond pools Sep 17, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35183458924. Please check Claude's inline feedback and summary comment for details.

@lanyue-llk

Copy link
Copy Markdown
Contributor Author

补充复查修复:release 原先用相对租约的 app_id 变化推断重置完成,但任务准备本身也会重置,可能误放行任务修改后的环境。现在释放前持久化 release_from_app_id,只认可本次释放发起的替换;运行配置写入或租约删除失败后可续办,不重复重建。新增任务准备后释放、配置写入失败重试的回归,并保留释放期间其他槽位可并发分配的测试。也支持 minimum_ready=0 缩到零,活跃租约仍阻止删除。相关环境/CLI/槽位测试共 64 项通过。此前 review 提到写入失败必然泄漏实例不准确(reset 会删除前一个实例),但重复重置确实不必要,本次一起修正。其余历史有效意见均已修复;没有修改已发布评测分数。

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

PR #18 Code Review

  • Gather context
  • Read changed files
  • Identify bugs and security issues
  • Post findings

View job run

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

PR #18 Code Review

  • 读取 AGENTS.md
  • 读取关键变更文件
  • 独立分析当前代码状态
  • 发布 review 结论

View job run

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 5m 37s —— View job


PR #18 Code Review

  • 读取 AGENTS.md
  • 读取关键变更文件
  • 独立分析当前代码状态
  • 发布 review 结论

Review 结论

通过

对最新两个 commit(fa1343f、1b423c2)逐行独立核查,前序各轮 review 报告的全部 P1/P2 均已修复,本次新增改动正确且有充分测试覆盖。

fa1343f 改动(Distinguish release cleanup from task preparation resets)核查:

  • release_slice() line 1052–1069:引入 release_from_app_id 标记,解决了旧逻辑用 app_id != lease["app_id"] 判断重置完成会误认任务准备阶段 _reset_impl() 触发的替换为释放完成的问题。新逻辑:首次调用时将 current_receipt["app_id"] 写入 release_from_app_id,只有本次释放发起的替换(current_receipt["app_id"] != release_from_app_id)才走早返回路径,直接写入 runtime config 并解锁,不重复 reset。逻辑正确,幂等性完整(config 写失败或 lease unlink 失败均可安全重试)。
  • scale_pool() line 892:minimum_ready 下限从 < 1 改为 < 0,允许缩到零。count=0, minimum_ready=0 时,range(1, 1) 为空集(不新建槽位),excess_slots 含全部现有槽位,活跃租约仍阻止删除(line 946–953)。逻辑正确。
  • 测试覆盖:test_release_does_not_mistake_task_preparation_for_cleanup 覆盖任务准备后释放场景;test_release_retry_only_unlinks_after_reset_was_persisted 参数化 failure_point=["unlink","config"] 分别覆盖 unlink 失败和 config 写入失败的重试路径;test_scale_down_is_atomic_when_an_excess_slice_is_leased 参数化 count=[0,1] 验证 minimum_ready=0 下活跃租约仍阻止删除。

1b423c2 改动(Validate every WebArena template against the canonical image lock)核查:

  • verify_webarena_env_contract.py:新增对 webarena.config.example.json 和 rainbond-pool.example.json 的 image_lock 路径校验,确保所有模板引用同一 canonical lock;同时验证 delivery contract 组件不得含独立 image 字段(必须从 lock 派生)。逻辑正确。
  • webarena.config.example.json:image_lock 路径从 ../../evaluation/...(错误,指向 examples/evaluation/...)改为 ../../../evaluation/...(正确,从 examples/trajectory_replay/public_webarena/ 上溯三级到仓库根)。修复前已被 test_evaluation_templates_reject_wrong_relative_lock_path 捕获验证。
  • 测试覆盖:test_repository_contract_uses_one_image_lock 使用实际文件验证通过;test_evaluation_templates_reject_wrong_relative_lock_path 参数化 ["webarena.config","rainbond-pool"] 验证路径错误时正确拒绝;test_deployment_rejects_duplicate_image_identity 验证组件含 image 字段时正确拒绝。

已独立确认历次修复点(与上轮 review 一致):

  • _attempt_cleanup 覆盖所有清理路径,原始异常不被覆盖。
  • _configure_component()(line 385–386)和 _configure_initializer()(line 468–469)对空 actual_image 显式 raise。
  • acquire_slice() 使用 _slot_sort_key 数值排序;provision_pool() 返回值同用。
  • _reset_impl() 调用 persist_runtime_config() 原子落盘。
  • _pool_lock() 同时持有 threading.Lock 和 fcntl.flock。
  • 凭据隔离正确:token_env 仅存变量名,receipt 不含 token,.env.example 符合 AGENTS.md 要求。

问题及建议

未发现需要阻塞合并的问题。残余风险:

  • _coalesce_internal_ports 重试间隔硬编码(1s/2s,最多 3 次),Rainbond 读模型延迟 > 3s 时可能偶发误报;已有注释说明,生产中需关注。
  • runtime_config() 将 image_lock 和 pool_config 绝对路径写入 config,仅同机器间有效;文档已说明,当前部署模型下可接受。
  • stop_before_replace 策略下若 provision() 失败,旧 app 已停止但不会自动重启;默认策略 replace_before_destroy 不受影响。

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Claude finished @lanyue-llk's task in 3m 46s —— View job


PR #18 Code Review

  • Gather context
  • Read changed files
  • Identify bugs and security issues
  • Post findings

All previously flagged bugs are confirmed fixed. No new issues found in the latest two commits (fa1343f, 1b423c2).

Confirmed new changes are correct:

  • release_slice now persists release_from_app_id into the lease file before calling pool.reset(). On retry, if current_receipt["app_id"] != release_from_app_id, it recognises the reset as complete and takes the short path (validate receipt, write config, unlink lease). This correctly distinguishes task-preparation resets (which happen under the lease before release_slice is called) from the release's own reset. The failure modes are safe: if _atomic_json(lease_path, lease) raises, pool.reset() is never reached, so a retry records release_from_app_id again and retries the reset correctly.

  • scale_pool validation changed from minimum_ready < 1 to minimum_ready < 0, enabling scale-to-zero (count=0, minimum_ready=0). Active leases still block scale-down (RuntimeError raised before any destroy calls). The existing excess-slot destroy loop uses _attempt_cleanup pattern, so a single failed destroy doesn't block others.

  • verify_webarena_env_contract.py now validates rainbond-pool.example.json alongside the other three templates, ensuring all four reference the canonical image lock.

No security issues found.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Automated Claude review completed. Review run: https://github.com/lexmount/browser-eval/actions/runs/35184318971. Please check Claude's inline feedback and summary comment for details.

@lanyue-llk
lanyue-llk merged commit 7520ed3 into main Sep 17, 2026
3 checks passed
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.

1 participant