[rollout] feat: add native Harbor backend - #267
Conversation
harbor_skypilot_env (terminal-bench-native rollout, GCP SkyPilot sandboxes) imports environment_content_hash / tar_transfer, first available in harbor >=0.16.1. The old >=0.15.0,<0.16.0 cap forced a resolve conflict against the rollout's harbor>=0.16.1. Widen the floor; NativeHarborBackend verified to import on 0.16.1 and 0.18.0. Co-authored-by: Cursor <cursoragent@cursor.com>
JoyboyBrian
added
enhancement
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
No issues found across 15 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Re-trigger cubic
There was a problem hiding this comment.
3 issues found across 18 files (changes from recent commits).
Confidence score: 3/5
- In
osmosis_ai/rollout/server/native_harbor_gateway.py, the SSE wrapper emits onlydata:frames, so clients routing on event names can missresponse.createdand delta events, breaking real-time parsing and downstream handlers — preserve and forward each upstream SSEevent:field alongside payloads. - In
osmosis_ai/rollout/backend/base.py, HTTP admission is still unbounded whileLocalBackendhas finite workers, which can let request queues grow until memory pressure rises or controller deadlines expire before execution — add an admission cap/backpressure path aligned with pool and queue limits. - In
docs/native-harbor-backend.md,/healthdocuments acapacityblock thathealth()does not currently return, which can mislead operators and automation that depend on those fields — either implement the documented capacity snapshot or update the docs to match current behavior.
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="osmosis_ai/rollout/server/native_harbor_gateway.py">
<violation number="1" location="osmosis_ai/rollout/server/native_harbor_gateway.py:122">
P2: Responses streaming clients that dispatch on SSE event names will not receive `response.created`, `response.output_text.delta`, or other event types because this wrapper emits only `data:` frames. Preserve each event's `type` as an `event:` line when forwarding the stream.</violation>
</file>
<file name="osmosis_ai/rollout/backend/base.py">
<violation number="1" location="osmosis_ai/rollout/backend/base.py:33">
P2: Local rollouts remain unbounded at the HTTP admission layer even though `LocalBackend` has a finite execution pool, allowing enough pending requests to grow memory usage or expire controller deadlines while waiting. A finite backend such as `LocalBackend` should override `max_queue_depth` (or the default should derive a bound from `max_concurrency`) rather than inheriting this unbounded fallback.</violation>
</file>
<file name="docs/native-harbor-backend.md">
<violation number="1" location="docs/native-harbor-backend.md:449">
P2: The documented /health response includes a `capacity` block with live fields (`in_flight`, `queue_depth`, `available`, `accepting`) that the backend's `health()` method does not return. If this capacity snapshot is intended, it must be implemented; otherwise remove the block from the doc example and the surrounding claim to avoid misleading readers about available/accepting admission state.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| try: | ||
| if hasattr(response, "__aiter__"): | ||
| async for event in response: | ||
| yield f"data: {_response_event_json(event)}\n\n" |
There was a problem hiding this comment.
P2: Responses streaming clients that dispatch on SSE event names will not receive response.created, response.output_text.delta, or other event types because this wrapper emits only data: frames. Preserve each event's type as an event: line when forwarding the stream.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At osmosis_ai/rollout/server/native_harbor_gateway.py, line 122:
<comment>Responses streaming clients that dispatch on SSE event names will not receive `response.created`, `response.output_text.delta`, or other event types because this wrapper emits only `data:` frames. Preserve each event's `type` as an `event:` line when forwarding the stream.</comment>
<file context>
@@ -0,0 +1,197 @@
+ try:
+ if hasattr(response, "__aiter__"):
+ async for event in response:
+ yield f"data: {_response_event_json(event)}\n\n"
+ else:
+ for event in response:
</file context>
| execution pool can override this to make ``POST /rollout`` fail fast | ||
| instead of letting controller deadlines expire in an unbounded queue. | ||
| """ | ||
| return None |
There was a problem hiding this comment.
P2: Local rollouts remain unbounded at the HTTP admission layer even though LocalBackend has a finite execution pool, allowing enough pending requests to grow memory usage or expire controller deadlines while waiting. A finite backend such as LocalBackend should override max_queue_depth (or the default should derive a bound from max_concurrency) rather than inheriting this unbounded fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At osmosis_ai/rollout/backend/base.py, line 33:
<comment>Local rollouts remain unbounded at the HTTP admission layer even though `LocalBackend` has a finite execution pool, allowing enough pending requests to grow memory usage or expire controller deadlines while waiting. A finite backend such as `LocalBackend` should override `max_queue_depth` (or the default should derive a bound from `max_concurrency`) rather than inheriting this unbounded fallback.</comment>
<file context>
@@ -22,5 +22,20 @@ def max_concurrency(self) -> int:
+ execution pool can override this to make ``POST /rollout`` fail fast
+ instead of letting controller deadlines expire in an unbounded queue.
+ """
+ return None
+
+ @property
</file context>
| } | ||
| ``` | ||
|
|
||
| Controllers currently ignore the additional health fields, so capacity and |
There was a problem hiding this comment.
P2: The documented /health response includes a capacity block with live fields (in_flight, queue_depth, available, accepting) that the backend's health() method does not return. If this capacity snapshot is intended, it must be implemented; otherwise remove the block from the doc example and the surrounding claim to avoid misleading readers about available/accepting admission state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/native-harbor-backend.md, line 449:
<comment>The documented /health response includes a `capacity` block with live fields (`in_flight`, `queue_depth`, `available`, `accepting`) that the backend's `health()` method does not return. If this capacity snapshot is intended, it must be implemented; otherwise remove the block from the doc example and the surrounding claim to avoid misleading readers about available/accepting admission state.</comment>
<file context>
@@ -93,82 +185,282 @@ Each row points at a Harbor task through a first-class `metadata` key. The datas
+}
+```
+
+Controllers currently ignore the additional health fields, so capacity and
+protocol surfacing are forward-compatible rather than full negotiation. The
+real Miles/eval capacity-mismatch measurement remains an E2E dependency.
</file context>
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
3 participants
What
NativeHarborBackendfor running one native Harbor trial per rollout with local, package, or Git task resolution, bounded concurrency, controller endpoint injection, and Docker, Daytona, and SkyPilot support.agent.extraleaves and overlay controller-reported metrics.Why
Native Harbor backend development paused before the SDK gained the single-sample URL-routed protocol, ATIF persistence, file artifacts, output downloads, and newer Harbor runtime support. Replaying the old branch diff would miss those contracts and could either discard Harbor's richer native trajectory or delete collected artifacts during cleanup. This brings the backend forward against the current SDK architecture while keeping Harbor responsible for selecting and downloading sandbox artifacts.
Tracks OSM-1653.
How to Test
uv run ruff check .uv run ruff format --check .uv run pyright osmosis_ai/uv run pytestuv run pytest tests/unit/rollout/test_native_harbor_backend.py tests/unit/cli/test_native_preflight.py tests/unit/rollout/test_trajectory_save.py -qChecklist
[module] type: descriptionformatenhancement,bug,breaking)ruff check .andruff format --check .passpyright osmosis_ai/passespytestpasses (new tests added if applicable)Summary by cubic
Adds a
NativeHarborBackendto run one nativeharbortrial per rollout with URL-identity routing, native ATIF persistence, a protocol gateway, and bounded server admission. Improves reliability and preflight, archives native diagnostics, and stabilizes Responses streaming. Tracks OSM-1653.New Features
NativeHarborBackendwith bounded concurrency viaTrialQueueand server admission (max_queue_depth); Docker/Daytona/SkyPilot support; controller URL/key injected from rollout context (no per-call headers).metadata["harbor_task"](local/package/git) viaresolve_task, optionalmetadata["harbor_model"]; map the task verifier’s reward to the single sample; reject rewards from failed trials; disable native retries.harbor-authored ATIF (process-local), redact sensitiveagent.extra, overlay controller metrics; copy onlyharbor-collected artifacts; keep trials if outputs can’t be safely saved; post and archive native diagnostics viaresult.extra_fields(sidecar when no sample).Migration
metadata["harbor_task"];system_prompt/user_promptare ignored. Pin task refs for reproducibility.AgentWorkflow/Grader; theharbortask defines instruction, environment, and reward. The server captures the final result without a grader URL.terminus-2with summarization disabled; override viaagent_kwargsif needed.Written for commit e0e5baf. Summary will update on new commits.