Repository navigation
Conversation
…env var computer-1 starts its own Xvfb, XFCE and Chromium and treats Chromium's exit as the session's end, so it can only drive a browser. HARBOR_CUA_DESKTOP=existing patches it, on a trial's first run, to use the image's display on :1: no desktop or browser of its own, liveness from the X socket, no browser reset or recovery, zoom and the browser actions refused and cut from the generic provider's prompt, a settle after each screen-changing action, PNG screenshots optionally resized in the sandbox, and a cap on the screenshots one request carries. It touches private Harbor names pinned at harbor-miles-v0.20.0, and is a stopgap until computer-1 takes desktop options.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73aff9f866
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| script = ( | ||
| f"set -e; export DISPLAY={runtime._DEFAULT_DISPLAY}; " | ||
| f"mkdir -p {shlex.quote(runtime._SCREENSHOT_DIR)} {shlex.quote(str(PurePosixPath(target).parent))}; " | ||
| f"import -window root {png}{resize}; cp {png} {shlex.quote(target)}" |
There was a problem hiding this comment.
Propagate capture failures before copying the screenshot
When HARBOR_CUA_SCREENSHOT_SIZE is set and import fails after an earlier successful capture, this becomes import … && convert …; cp …. Although Bash's help set describes -e as “Exit immediately if a command exits with a non-zero status,” a nonfinal command in an && list is exempt: execution proceeds to cp, which can successfully copy the previous latest.png. I reproduced this control flow locally with a failing capture command and an existing file: it returned status 0 and copied the old contents. The agent therefore receives a stale screen as a successful new observation, potentially corrupting the rollout. Make capture, resize, and copy one failure-propagating chain, or explicitly check the capture status.
Useful? React with 👍 / 👎.
Layer P24 of the CUA upstreaming stack: Harbor computer-1 on a desktop its image already runs, as an env-gated monkeypatch with no new class. It is built and reviewed in celve, and goes upstream with the whole stack once every layer is finished. Base: P17's
LIN-2524/harbor-computer1-session-server(#39,6f19707331). Issue: LIN-2712, hub LIN-2504.Problem. Harbor's
computer-1starts its own Xvfb, XFCE, VNC and Chromium, judges liveness bypgrep chromium, and relaunches Chromium after a failed action, so it can only drive a browser. A task image that already runs a desktop on:1(CUA-Lite's GNOME template, OSWorld-style images) gets a second desktop and a browser on top of it, and a session that ends whenever Chromium is absent.Computer1.__init__refuses unknown kwargs, so there is no way to pass settings for this either. Every measured CUA run (LIN-2149, LIN-2399, LIN-2631) used our fork'sDesktopQwen3VLComputer1subclass (#9) to get around this.Fix.
examples/experimental/harbor/harbor_agent_function.pygainsapply_existing_desktop(), modeled on P17'sapply_coordinate_space(): applied once per process on the firstrun()(Harbor is imported lazily there), unset leaves Harbor untouched, any value other thanexistingstops the run. WithHARBOR_CUA_DESKTOP=existingit patches:Computer1Session.start: create the runtime dirs and wait for the X socket; no Xvfb, XFCE, VNC or Chromium.Computer1Session.is_session_alive: the X socket only.Computer1Session.resetraises and_recover_chromium_if_neededreturnsNone: there is no browser to relaunch.Computer1Session.execute: refuseszoom,navigate,go_backandgo_forward(the desktop capture does no crop, and there is no URL bar), then waitsHARBOR_CUA_SETTLE_SEC(default 2.0) after every action that changes the screen (notwait,sleep,done,answer).Computer1Session.fetch_screenshot: a PNG fromimport -window root, resized in the sandbox toHARBOR_CUA_SCREENSHOT_SIZE=WxHwhen set.GenericJsonProvider.from_agent: wraps whatever is there (so it composes with P17's grid patch), setsscreenshot_format = "png", and uses Harbor'scomputer-1-json.txtwith the zoom/navigate lines cut. A cut line that is no longer in the template exactly once stops the run at patch time.HARBOR_CUA_KEEP_SCREENSHOTS=N:Computer1._query_litellmandComputer1._fallback_message_historytrim the history with Harbor's ownComputer1Compactor._trim_old_screenshots, counting the images the prompt itself carries, so a request holds at most N, the current one included (a JSON-parse retry keeps the current screen; the turn-cap fallback stays within the cap).launch_common.harbor_env_varsforwards the four variables; the module header and README document them. The patch is three helpers plus the gate, about 125 lines (the whole layer is +368/−3, source +184/−2).This patch is a stopgap. A Harbor PR adding desktop kwargs to
computer-1(use the image's display, settle, screenshot size, screenshot cap) on harbor-framework'sharbor-miles-v0.20.0would replace it; that PR is not started. Unlike P17's patch, this one touches private Harbor names, so it pinsharbor-miles-v0.20.0(c699a7e) and a contract test names each one:runtime:_RUNTIME_DIR,_SCREENSHOT_DIR,_DEFAULT_DISPLAY,_bash_inline;Computer1Session._recover_chromium_if_needed,_wait_for_x11,_exec,_raw_exec,_started,_agent_dir,_request_timeout_sec.providers.generic:_TEMPLATES_DIR,GenericJsonProvider._prompt_template.Computer1:_query_litellm,_fallback_message_history,_compactor._trim_old_screenshots,_enable_images,_latest_screenshot_path.Public names patched:
Computer1Session.start,is_session_alive,reset,execute,fetch_screenshot,GenericJsonProvider.from_agent,screenshot_format.Tests (fresh EC2 CPU box, Miles venv,
harbor-miles-v0.20.0installed into a private target dir, P15 shim onPYTHONPATH), on73aff9f866:pytest tests/fast/examples/experimental/harborwith real Harbor: 96 passed (86 on the base). Without Harbor: 59 passed, 1 skipped (the contract module).AgentFactoryand driving its real session against a fake environment that records everyexec: setup issues only the mkdir and the X-socket wait (no Xvfb, XFCE, x11vnc or Chromium); liveness runs only the X-socket test, recovery returnsNone, reset raises;navigateandzoomare refused, a click settles 0.25 s and awaitadds no settle; the screenshot command isimport -window root … && convert … -resize 1280x720! …, a.webptarget is refused, and the provider prompts without navigate/zoom and asks for PNG; withKEEP_SCREENSHOTS=2, a request against a local chat server over three earlier screens plus the current one carries 2 images and the fallback history 1; unset leaves all nine patched attributes identical; three bad settings raise; every private name above exists.harbor_agent_function.pyback at the base plus an emptyapply_existing_desktop(): 8 of the new contract tests fail (all five behaviour tests and the three bad-setting cases); the unchanged-Harbor and names tests pass, as they should.pre-commit runon the changed files: passed.Limits.
lin2139-liteosworld, and which LIN-2399 and LIN-2631 trained with; this port has only the contract tests above.import/convertandxdotoolon its PATH. Run Harbor computer-1 on a desktop its image already runs #9's_capture_script/session_classhooks for images without ImageMagick are dropped (they were subclass hooks).HARBOR_CUA_SCREENSHOT_SIZEonly shrinks the image; clicks still go through Harbor'sdesktop_width/desktop_height, so pair a resize with P17'sHARBOR_CUA_COORDINATE_SPACE=normalized_0_999or keep it at the desktop's size.SandboxLostdiscard, as in P17.Related upstream work. No open upstream Harbor or Miles PR runs computer-1 on an existing desktop. radixark/miles radixark#2806 (Tao Lin, nblintao, merged) is the in-process Harbor agent function this extends; P17 (#39) is the computer-1 binding and the grid patch this composes with.
Origin commits: #9 (
09564ac8e2) onLIN-1985/investigate-existing-recipes-for,examples/experimental/harbor_cua/desktop_computer.py(DesktopSession,DesktopJsonProvider,DesktopQwen3VLComputer1), rewritten as functions patching Harbor's own classes; the behaviour is ported, the classes are not.