Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openhands/automation/capabilities_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@
"presetPlugin",
"presetPrompt",
"repoClone",
# A preset's repositories can be changed with PATCH after creation.
"repoUpdate",
)

# Tags Pydantic inserts into an error location for the trigger union.
Expand Down
2 changes: 1 addition & 1 deletion openhands/automation/git_sync/loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ async def _delete_superseded_upload(
Without this, every git-side edit left the previous upload row and file
referenced by nothing, so routine PR edits accumulated a tarball copy per
merge with no reaper to collect them. Mirrors the cleanup
`regenerate_preset_prompt_tarball` does for prompt edits.
`regenerate_preset_tarball` does for prompt and repository edits.

Soft-deletes the record in the current transaction; the storage object is
only queued on `pending_storage_deletes` and removed after the cycle's
Expand Down
87 changes: 57 additions & 30 deletions openhands/automation/preset_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import os
import tarfile
import uuid
from collections.abc import AsyncIterator
from collections.abc import AsyncIterator, Mapping
from pathlib import Path
from typing import Any

Expand Down Expand Up @@ -305,41 +305,57 @@ def _generate_tarball(prompt: str, repos: list[RepoSource] | None = None) -> byt

# Add repos config if repos specified (SDK workspace handles cloning)
if repos:
repos_config = [r.model_dump(exclude_none=True) for r in repos]
_add_file_to_tar(
tar, "repos_config.json", json.dumps(repos_config, indent=2)
tar, "repos_config.json", repos_config_json(dump_repos(repos))
)

tarball_buffer.seek(0)
return tarball_buffer.read()


def dump_repos(repos: list[RepoSource] | None) -> list[dict[str, Any]]:
"""Repositories as presets record them in ``preset_metadata.repos``."""
return [r.model_dump(exclude_none=True) for r in repos or []]


def repos_config_json(repos: list[dict[str, Any]]) -> str:
"""The ``repos_config.json`` a preset tarball carries, from ``dump_repos``."""
return json.dumps(repos, indent=2)


# Alias kept for existing call sites; canonical impl now in tarball_validation.
_build_storage_path = build_upload_storage_path


def _replace_prompt_in_tarball(tarball_bytes: bytes, new_prompt: str) -> bytes | None:
"""Return a copy of a preset tarball with ``prompt.txt`` swapped for ``new_prompt``.
def _replace_files_in_tarball(
tarball_bytes: bytes, files: Mapping[str, str | None]
) -> bytes | None:
"""Return a copy of a preset tarball with the named files replaced.

Every other member (``main.py``, ``setup.sh``, ``plugins_config.json``,
``repos_config.json``, ...) is copied through unchanged, so plugin and repo
configuration are preserved and the working template is untouched.
A ``None`` value removes that file, and a name the archive lacks is added.
Every other member (``main.py``, ``setup.sh``, ``plugins_config.json``, ...)
is copied through unchanged, so the working template is untouched.

Returns ``None`` if the archive has no ``prompt.txt`` member — i.e. it is not a
regenerable preset tarball — so the caller can leave the tarball as-is.
"""
out_buffer = io.BytesIO()
found = False
replaced: set[str] = set()
with (
tarfile.open(fileobj=io.BytesIO(tarball_bytes), mode="r:gz") as src,
tarfile.open(fileobj=out_buffer, mode="w:gz") as dst,
):
for member in src.getmembers():
if member.name == "prompt.txt":
found = True
_add_file_to_tar(
dst, "prompt.txt", new_prompt, mode=member.mode or 0o644
)
if member.name in files:
replaced.add(member.name)
content = files[member.name]
if content is not None:
_add_file_to_tar(
dst, member.name, content, mode=member.mode or 0o644
)
continue
if member.isfile():
extracted = src.extractfile(member)
Expand All @@ -351,6 +367,9 @@ def _replace_prompt_in_tarball(tarball_bytes: bytes, new_prompt: str) -> bytes |
dst.addfile(info, io.BytesIO(data))
else:
dst.addfile(member)
for name, content in files.items():
if name not in replaced and content is not None:
_add_file_to_tar(dst, name, content)

if not found:
return None
Expand All @@ -371,22 +390,27 @@ def _delete_storage_object_best_effort(
logger.exception("Failed to delete superseded tarball at %s", storage_path)


async def regenerate_preset_prompt_tarball(
_PRESET_FILE_LABELS = {"prompt.txt": "Prompt", "repos_config.json": "Repositories"}


async def regenerate_preset_tarball(
automation: Automation,
new_prompt: str,
files: Mapping[str, str | None],
session: AsyncSession,
background_tasks: BackgroundTasks,
) -> str | None:
"""Rebuild a preset automation's tarball with an updated prompt.

Preset automations bake the prompt into ``prompt.txt`` inside the tarball the
dispatcher executes; the stored ``prompt`` column is metadata only. When the
prompt is edited the tarball must be rewritten too, otherwise dispatching keeps
running the original prompt.

Reads the automation's current internal-upload tarball, swaps in ``new_prompt``
(leaving all other files untouched), uploads the result as a new internal upload,
and returns its ``oh-internal://`` URL for the caller to store on ``tarball_path``.
"""Rebuild a preset automation's tarball with updated files.

Preset automations bake the prompt into ``prompt.txt`` and their repositories
into ``repos_config.json`` inside the tarball the dispatcher executes; the
stored ``prompt`` column and ``preset_metadata`` are a record only. When either
is edited the tarball must be rewritten too, otherwise dispatching keeps running
the original files.

Reads the automation's current internal-upload tarball, replaces ``files``
(``None`` removes a file; all other files are untouched), uploads the result as
a new internal upload, and returns its ``oh-internal://`` URL for the caller to
store on ``tarball_path``.
The superseded upload is soft-deleted in the current transaction; its storage
object is removed via ``background_tasks`` only after the transaction commits.

Expand All @@ -412,23 +436,26 @@ async def regenerate_preset_prompt_tarball(
except ObjectNotFoundError:
# Only confirmed absence means "not regenerable"; a transient storage
# error must propagate (rolling back the edit) rather than silently
# leaving the old prompt baked into the tarball.
# leaving the old files baked into the tarball.
return None

new_tarball = _replace_prompt_in_tarball(current_tarball, new_prompt)
new_tarball = _replace_files_in_tarball(current_tarball, files)
if new_tarball is None:
return None

new_upload_id = uuid.uuid4()
storage_path = _build_storage_path(
automation.org_id, automation.user_id, new_upload_id
)
changed = " and ".join(
_PRESET_FILE_LABELS.get(name, name) for name in sorted(files)
)
upload = TarballUpload(
id=new_upload_id,
user_id=automation.user_id,
org_id=automation.org_id,
name=f"prompt-automation-{_safe_truncate(automation.name, 50)}-edit",
description=f"Prompt updated for: {_safe_truncate(automation.name, 100)}",
description=f"{changed} updated for: {_safe_truncate(automation.name, 100)}",
status=UploadStatus.UPLOADING,
storage_path=storage_path,
)
Expand Down Expand Up @@ -481,6 +508,7 @@ async def regenerate_preset_prompt_tarball(
"old_storage_path": source_upload.storage_path,
"new_upload_id": str(new_upload_id),
"new_storage_path": storage_path,
"files": sorted(files),
},
)

Expand Down Expand Up @@ -576,7 +604,7 @@ async def create_automation_from_prompt(
"prompt": body.prompt,
}
if body.repos:
preset_metadata["repos"] = [r.model_dump(exclude_none=True) for r in body.repos]
preset_metadata["repos"] = dump_repos(body.repos)
if body.template is not None:
preset_metadata["template"] = body.template.model_dump(exclude_none=True)

Expand Down Expand Up @@ -908,9 +936,8 @@ def _generate_plugin_tarball(
)

if repos:
repos_config = [r.model_dump(exclude_none=True) for r in repos]
_add_file_to_tar(
tar, "repos_config.json", json.dumps(repos_config, indent=2)
tar, "repos_config.json", repos_config_json(dump_repos(repos))
)

tarball_buffer.seek(0)
Expand Down Expand Up @@ -1040,7 +1067,7 @@ async def create_automation_from_plugin(
p.model_dump(exclude_none=True) for p in body.plugins
]
if body.repos:
preset_metadata["repos"] = [r.model_dump(exclude_none=True) for r in body.repos]
preset_metadata["repos"] = dump_repos(body.repos)
if body.template is not None:
preset_metadata["template"] = body.template.model_dump(exclude_none=True)

Expand Down
74 changes: 60 additions & 14 deletions openhands/automation/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@
validate_plugin_config,
validate_required_plugins,
)
from openhands.automation.preset_router import regenerate_preset_prompt_tarball
from openhands.automation.preset_router import (
dump_repos,
regenerate_preset_tarball,
repos_config_json,
)
from openhands.automation.schemas import (
AutomationListResponse,
AutomationResponse,
Expand Down Expand Up @@ -685,6 +689,18 @@ async def update_automation(

update_data = body.model_dump(exclude_unset=True)
_assert_can_update_fields(auto, user, update_data)
# Repositories are not a column: a preset bakes them into its tarball and
# records them in preset_metadata, so they are applied with the prompt below.
repos_requested = "repos" in update_data
update_data.pop("repos", None)
if repos_requested and (auto.preset_metadata or {}).get("preset_type") not in (
"prompt",
"plugin",
):
raise HTTPException(
status.HTTP_422_UNPROCESSABLE_CONTENT,
"Only prompt and plugin preset automations can update repos",
)
plugin_update_requested = "plugin" in update_data
if plugin_update_requested:
if update_data["plugin"] is None:
Expand Down Expand Up @@ -839,25 +855,51 @@ async def update_automation(
for field, value in update_data.items():
setattr(auto, field, value)

# A preset automation bakes its prompt into the tarball the dispatcher
# executes; the `prompt` column is metadata only. When the prompt actually
# changes, rebuild the tarball so the next dispatch runs the new prompt
# instead of the original baked one. Skipped when the value is unchanged (a
# no-op edit), or for non-preset automations.
if (
# A preset automation bakes its prompt and repositories into the tarball the
# dispatcher executes; the `prompt` column and preset metadata are a record
# only. When either actually changes, rebuild the tarball once so the next
# dispatch runs the edit instead of the original baked files. Skipped when
# the values are unchanged (a no-op edit), or for non-preset automations.
prompt_changed = (
"prompt" in update_data
and isinstance(auto.prompt, str)
and auto.prompt != original_prompt
):
new_tarball_path = await regenerate_preset_prompt_tarball(
auto, auto.prompt, session, background_tasks
)
new_repos = dump_repos(body.repos)
repos_changed = repos_requested and new_repos != (
(auto.preset_metadata or {}).get("repos") or []
)
files: dict[str, str | None] = {}
if prompt_changed:
files["prompt.txt"] = auto.prompt
if repos_changed:
files["repos_config.json"] = repos_config_json(new_repos) if new_repos else None
if files:
new_tarball_path = await regenerate_preset_tarball(
auto, files, session, background_tasks
)
if new_tarball_path is not None:
auto.tarball_path = new_tarball_path
# Keep preset metadata in sync with the edited prompt. Reassign the
# whole dict: in-place mutation of a JSON column is not change-tracked.
elif repos_changed:
# Unlike the prompt column, recorded repositories that the tarball
# does not clone would claim a checkout the next run never makes.
raise HTTPException(
status.HTTP_422_UNPROCESSABLE_CONTENT,
"This automation's repositories cannot be changed: its preset "
"tarball could not be rebuilt",
)
# Keep preset metadata in sync with the edit. Reassign the whole dict:
# in-place mutation of a JSON column is not change-tracked.
if auto.preset_metadata is not None:
auto.preset_metadata = {**auto.preset_metadata, "prompt": auto.prompt}
metadata = dict(auto.preset_metadata)
if prompt_changed:
metadata["prompt"] = auto.prompt
if repos_changed:
if new_repos:
metadata["repos"] = new_repos
else:
metadata.pop("repos", None)
auto.preset_metadata = metadata

if skip_pending_reason is not None:
await skip_pending_runs_for_disabled_automation(
Expand All @@ -879,7 +921,11 @@ async def update_automation(
request=request,
user=user,
automation=auto,
properties={"updated_fields": sorted(update_data.keys())},
properties={
"updated_fields": sorted(
update_data.keys() | ({"repos"} if repos_requested else set())
)
},
)
return AutomationResponse.model_validate(auto)

Expand Down
15 changes: 15 additions & 0 deletions openhands/automation/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
validate_automation_timeout,
)
from openhands.sdk.event.conversation_error import ConversationErrorEvent
from openhands.workspace import RepoSource


# Allowed URI schemes for tarball_path (includes internal upload scheme)
Expand Down Expand Up @@ -672,6 +673,14 @@ class UpdateAutomationRequest(BaseModel):
)

prompt: str | None = Field(default=None, max_length=50000)
repos: list[RepoSource] | None = Field(
default=None,
description=(
"Repositories a prompt or plugin preset clones before it runs. Replaces "
"the whole list; an empty list or null removes them. Other automations "
"answer 422."
),
)
trigger: Trigger | None = Field(
default=None, description="Trigger configuration (cron or event-based)"
)
Expand Down Expand Up @@ -719,6 +728,12 @@ def validate_tarball_path(cls, v: str | None) -> str | None:
)
return v

@field_validator("repos", mode="before")
@classmethod
def normalize_repos(cls, v: Any) -> Any:
"""Accept a single repository as well as a list, as the presets do."""
return [v] if isinstance(v, (str, dict)) else v

@field_validator("setup_script_path")
@classmethod
def validate_setup_script_path(cls, v: str | None) -> str | None:
Expand Down
1 change: 1 addition & 0 deletions tests/test_capabilities_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ async def test_configured_deployment_advertises_event_support(
assert "webhookDelivery" in body["features"]
assert "kvStore" in body["features"]
assert "customTarball" in body["features"]
assert "repoUpdate" in body["features"]

async def test_advertises_the_configured_timeout_ceiling(
self, async_client, ready_deployment, monkeypatch
Expand Down
Loading
Loading