From 9bdb37e7158636bc34f4817be2141ecc1ddba509 Mon Sep 17 00:00:00 2001 From: Alona <228736368+ak684@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:30:04 +0000 Subject: [PATCH] feat: let PATCH change a preset automation's repositories --- openhands/automation/capabilities_router.py | 2 + openhands/automation/git_sync/loop.py | 2 +- openhands/automation/preset_router.py | 87 +++-- openhands/automation/router.py | 74 ++++- openhands/automation/schemas.py | 15 + tests/test_capabilities_router.py | 1 + tests/test_preset_router.py | 56 +++- tests/test_router.py | 349 +++++++++++++++++++- 8 files changed, 533 insertions(+), 53 deletions(-) diff --git a/openhands/automation/capabilities_router.py b/openhands/automation/capabilities_router.py index 96fd53b4..1402c271 100644 --- a/openhands/automation/capabilities_router.py +++ b/openhands/automation/capabilities_router.py @@ -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. diff --git a/openhands/automation/git_sync/loop.py b/openhands/automation/git_sync/loop.py index 58c62764..78c69599 100644 --- a/openhands/automation/git_sync/loop.py +++ b/openhands/automation/git_sync/loop.py @@ -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 diff --git a/openhands/automation/preset_router.py b/openhands/automation/preset_router.py index 51e6023a..a925790e 100644 --- a/openhands/automation/preset_router.py +++ b/openhands/automation/preset_router.py @@ -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 @@ -305,31 +305,43 @@ 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, @@ -337,9 +349,13 @@ def _replace_prompt_in_tarball(tarball_bytes: bytes, new_prompt: str) -> bytes | 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) @@ -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 @@ -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. @@ -412,10 +436,10 @@ 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 @@ -423,12 +447,15 @@ async def regenerate_preset_prompt_tarball( 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, ) @@ -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), }, ) @@ -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) @@ -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) @@ -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) diff --git a/openhands/automation/router.py b/openhands/automation/router.py index 031ed01a..3bf250b9 100644 --- a/openhands/automation/router.py +++ b/openhands/automation/router.py @@ -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, @@ -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: @@ -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( @@ -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) diff --git a/openhands/automation/schemas.py b/openhands/automation/schemas.py index 959018da..f55c4c2f 100644 --- a/openhands/automation/schemas.py +++ b/openhands/automation/schemas.py @@ -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) @@ -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)" ) @@ -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: diff --git a/tests/test_capabilities_router.py b/tests/test_capabilities_router.py index 7fee3e2a..547bdf66 100644 --- a/tests/test_capabilities_router.py +++ b/tests/test_capabilities_router.py @@ -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 diff --git a/tests/test_preset_router.py b/tests/test_preset_router.py index 7d72c19a..fd9fb02a 100644 --- a/tests/test_preset_router.py +++ b/tests/test_preset_router.py @@ -22,7 +22,7 @@ _generate_plugin_tarball, _generate_tarball, _get_preset_entrypoint, - _replace_prompt_in_tarball, + _replace_files_in_tarball, _resolve_experiment_variant_models, ) from openhands.sdk.mcp.config import coerce_mcp_config, dump_mcp_config @@ -585,8 +585,8 @@ def test_generate_tarball_with_repos(self): assert repos_config[1]["ref"] == "main" -class TestReplacePromptInTarball: - """Tests for swapping prompt.txt inside an existing preset tarball.""" +class TestReplaceFilesInTarball: + """Tests for swapping files inside an existing preset tarball.""" def test_replaces_prompt_and_preserves_sibling_files(self): """The prompt is swapped while every other file is left byte-for-byte intact.""" @@ -599,7 +599,7 @@ def test_replaces_prompt_and_preserves_sibling_files(self): ) # Act - updated = _replace_prompt_in_tarball(original, "New prompt") + updated = _replace_files_in_tarball(original, {"prompt.txt": "New prompt"}) # Assert assert updated is not None @@ -640,7 +640,53 @@ def test_returns_none_when_tarball_has_no_prompt(self): tar.addfile(info, io.BytesIO(data)) # Act / Assert - assert _replace_prompt_in_tarball(buffer.getvalue(), "New prompt") is None + assert ( + _replace_files_in_tarball(buffer.getvalue(), {"prompt.txt": "New prompt"}) + is None + ) + + def test_adds_missing_file_and_preserves_others(self): + """A replacement the archive lacks is added; existing files are kept.""" + # Arrange — a prompt preset created without repositories. + original = _generate_tarball("Same prompt") + + # Act + updated = _replace_files_in_tarball( + original, {"repos_config.json": '[{"url": "owner/repo"}]'} + ) + + # Assert + assert updated is not None + with tarfile.open(fileobj=io.BytesIO(updated), mode="r:gz") as tar: + names = sorted(tar.getnames()) + repos_file = tar.extractfile("repos_config.json") + prompt_file = tar.extractfile("prompt.txt") + assert repos_file is not None and prompt_file is not None + assert json.loads(repos_file.read()) == [{"url": "owner/repo"}] + assert prompt_file.read().decode() == "Same prompt" + assert names == [ + "finish_tool_hook.py", + "main.py", + "prompt.txt", + "repos_config.json", + "setup.sh", + ] + + def test_none_removes_file(self): + """A ``None`` replacement drops that file from the archive.""" + # Arrange — a prompt preset that clones a repository. + original = _generate_tarball( + "Same prompt", repos=[RepoSource(url="owner/repo", provider="github")] + ) + + # Act + updated = _replace_files_in_tarball(original, {"repos_config.json": None}) + + # Assert + assert updated is not None + with tarfile.open(fileobj=io.BytesIO(updated), mode="r:gz") as tar: + assert "repos_config.json" not in tar.getnames() + assert "prompt.txt" in tar.getnames() class TestRepoSource: diff --git a/tests/test_router.py b/tests/test_router.py index bf3b361d..ea2d6fac 100644 --- a/tests/test_router.py +++ b/tests/test_router.py @@ -2,6 +2,7 @@ import contextlib import io +import json import re import tarfile import uuid @@ -21,13 +22,19 @@ TarballUpload, UploadStatus, ) -from openhands.automation.preset_router import _build_storage_path, _generate_tarball +from openhands.automation.preset_router import ( + _build_storage_path, + _generate_plugin_tarball, + _generate_tarball, +) from openhands.automation.storage import ObjectNotFoundError from openhands.automation.utils import utcnow from openhands.automation.utils.tarball_validation import ( build_internal_url, parse_internal_upload_id, ) +from openhands.sdk.plugin import PluginSource +from openhands.workspace import RepoSource # Test UUIDs matching mock_authenticated_user fixture @@ -99,11 +106,13 @@ async def _write_stream( return store -async def _seed_prompt_preset_automation(async_session, store, prompt): +async def _seed_prompt_preset_automation( + async_session, store, prompt, *, tarball=None, preset_metadata=None +): """Insert a prompt-preset automation whose internal tarball bakes ``prompt``.""" upload_id = uuid.uuid4() storage_path = _build_storage_path(TEST_ORG_ID, TEST_USER_ID, upload_id) - store._storage[storage_path] = _generate_tarball(prompt) + store._storage[storage_path] = tarball or _generate_tarball(prompt) async_session.add( TarballUpload( @@ -124,12 +133,29 @@ async def _seed_prompt_preset_automation(async_session, store, prompt): tarball_path=build_internal_url(upload_id), setup_script_path="setup.sh", entrypoint=".venv/bin/python main.py", + preset_metadata=preset_metadata, ) async_session.add(automation) await async_session.commit() return automation +def _tarball_files(store, tarball_path): + """Read every file of an internal tarball the preset store holds.""" + upload_id = parse_internal_upload_id(tarball_path) + assert upload_id is not None + storage_path = _build_storage_path(TEST_ORG_ID, TEST_USER_ID, upload_id) + files = {} + with tarfile.open( + fileobj=io.BytesIO(store._storage[storage_path]), mode="r:gz" + ) as tar: + for member in tar.getmembers(): + extracted = tar.extractfile(member) + if extracted is not None: + files[member.name] = extracted.read() + return files + + @pytest.fixture def _automation_for_permission_tests(async_session): """Create an automation owned by the test user for permission tests.""" @@ -1925,6 +1951,323 @@ async def test_update_prompt_syncs_preset_metadata( "repos": [{"url": "owner/repo"}], } + async def test_update_repos_rebuilds_preset_tarball( + self, async_client, async_session, preset_store + ): + """Editing repos rewrites repos_config.json and the recorded repos.""" + # Arrange — a prompt preset that clones one repository. + automation = await _seed_prompt_preset_automation( + async_session, + preset_store, + "Same prompt", + tarball=_generate_tarball( + "Same prompt", repos=[RepoSource(url="owner/old", provider="github")] + ), + preset_metadata={ + "preset_type": "prompt", + "prompt": "Same prompt", + "repos": [{"url": "owner/old", "provider": "github"}], + }, + ) + original_tarball_path = automation.tarball_path + + # Act — replace the repository and pin a branch. + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", + json={"repos": [{"url": "owner/new", "provider": "github", "ref": "dev"}]}, + ) + + # Assert — the record and the executable tarball both carry the new list. + assert response.status_code == 200 + data = response.json() + new_repos = [{"url": "owner/new", "ref": "dev", "provider": "github"}] + assert data["preset_metadata"] == { + "preset_type": "prompt", + "prompt": "Same prompt", + "repos": new_repos, + } + assert data["tarball_path"] != original_tarball_path + files = _tarball_files(preset_store, data["tarball_path"]) + assert json.loads(files["repos_config.json"]) == new_repos + assert files["prompt.txt"].decode() == "Same prompt" + + async def test_update_repos_accepts_a_single_repository( + self, async_client, async_session, preset_store + ): + """A single repository object is accepted as a one-item list.""" + # Arrange — a prompt preset created without repositories. + automation = await _seed_prompt_preset_automation( + async_session, + preset_store, + "Same prompt", + preset_metadata={"preset_type": "prompt", "prompt": "Same prompt"}, + ) + + # Act + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", + json={"repos": {"url": "owner/repo", "provider": "github"}}, + ) + + # Assert — the missing repos_config.json is added. + assert response.status_code == 200 + data = response.json() + assert data["preset_metadata"]["repos"] == [ + {"url": "owner/repo", "provider": "github"} + ] + files = _tarball_files(preset_store, data["tarball_path"]) + assert json.loads(files["repos_config.json"]) == [ + {"url": "owner/repo", "provider": "github"} + ] + + async def test_update_repos_empty_list_removes_them( + self, async_client, async_session, preset_store + ): + """An empty list removes the repositories from the record and tarball.""" + # Arrange + automation = await _seed_prompt_preset_automation( + async_session, + preset_store, + "Same prompt", + tarball=_generate_tarball( + "Same prompt", repos=[RepoSource(url="owner/old", provider="github")] + ), + preset_metadata={ + "preset_type": "prompt", + "prompt": "Same prompt", + "repos": [{"url": "owner/old", "provider": "github"}], + }, + ) + + # Act + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", json={"repos": []} + ) + + # Assert + assert response.status_code == 200 + data = response.json() + assert data["preset_metadata"] == { + "preset_type": "prompt", + "prompt": "Same prompt", + } + files = _tarball_files(preset_store, data["tarball_path"]) + assert "repos_config.json" not in files + assert files["prompt.txt"].decode() == "Same prompt" + + async def test_update_prompt_and_repos_rebuilds_tarball_once( + self, async_client, async_session, preset_store + ): + """Editing the prompt and repos together writes one new tarball.""" + # Arrange + automation = await _seed_prompt_preset_automation( + async_session, + preset_store, + "Original prompt", + preset_metadata={"preset_type": "prompt", "prompt": "Original prompt"}, + ) + + # Act + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", + json={ + "prompt": "Updated prompt", + "repos": [{"url": "owner/repo", "provider": "github"}], + }, + ) + + # Assert — one upload carries both edits. + assert response.status_code == 200 + assert preset_store.write_stream.await_count == 1 + data = response.json() + assert data["preset_metadata"] == { + "preset_type": "prompt", + "prompt": "Updated prompt", + "repos": [{"url": "owner/repo", "provider": "github"}], + } + files = _tarball_files(preset_store, data["tarball_path"]) + assert files["prompt.txt"].decode() == "Updated prompt" + assert json.loads(files["repos_config.json"]) == [ + {"url": "owner/repo", "provider": "github"} + ] + + async def test_update_unchanged_repos_does_not_regenerate_tarball( + self, async_client, async_session, preset_store + ): + """Re-sending the current repositories is a no-op.""" + # Arrange + automation = await _seed_prompt_preset_automation( + async_session, + preset_store, + "Same prompt", + tarball=_generate_tarball( + "Same prompt", repos=[RepoSource(url="owner/repo", provider="github")] + ), + preset_metadata={ + "preset_type": "prompt", + "prompt": "Same prompt", + "repos": [{"url": "owner/repo", "provider": "github"}], + }, + ) + original_tarball_path = automation.tarball_path + + # Act + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", + json={"repos": [{"url": "owner/repo", "provider": "github"}]}, + ) + + # Assert + assert response.status_code == 200 + assert response.json()["tarball_path"] == original_tarball_path + preset_store.write_stream.assert_not_called() + + async def test_update_repos_on_plugin_preset_keeps_plugins( + self, async_client, async_session, preset_store + ): + """A plugin preset's repositories change without touching its plugins.""" + # Arrange + original = _generate_plugin_tarball( + [PluginSource(source="github:owner/plugin")], "Run the plugin" + ) + automation = await _seed_prompt_preset_automation( + async_session, + preset_store, + "Run the plugin", + tarball=original, + preset_metadata={ + "preset_type": "plugin", + "prompt": "Run the plugin", + "plugins": [{"source": "github:owner/plugin"}], + }, + ) + + # Act + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", + json={"repos": [{"url": "owner/repo", "provider": "github"}]}, + ) + + # Assert + assert response.status_code == 200 + files = _tarball_files(preset_store, response.json()["tarball_path"]) + assert json.loads(files["repos_config.json"]) == [ + {"url": "owner/repo", "provider": "github"} + ] + with tarfile.open(fileobj=io.BytesIO(original), mode="r:gz") as tar: + plugins_file = tar.extractfile("plugins_config.json") + assert plugins_file is not None + assert files["plugins_config.json"] == plugins_file.read() + + @pytest.mark.parametrize( + "preset_metadata", + [None, {"template": {"id": "bundle"}}, {"preset_type": "plugin_spec"}], + ) + async def test_update_repos_on_other_automations_returns_422( + self, async_client, async_session, preset_metadata + ): + """Only prompt and plugin presets clone repositories they own.""" + # Arrange — a custom bundle, a template bundle and a plugin spec. + automation = Automation( + user_id=TEST_USER_ID, + org_id=TEST_ORG_ID, + name="Not A Preset", + trigger={"type": "cron", "schedule": "0 9 * * *", "timezone": "UTC"}, + tarball_path="s3://bucket/code.tar.gz", + entrypoint="uv run script.py", + preset_metadata=preset_metadata, + ) + async_session.add(automation) + await async_session.commit() + + # Act + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", + json={"repos": [{"url": "owner/repo", "provider": "github"}]}, + ) + + # Assert + assert response.status_code == 422 + assert response.json()["detail"] == ( + "Only prompt and plugin preset automations can update repos" + ) + + async def test_update_repos_rejects_an_invalid_repository( + self, async_client, async_session, preset_store + ): + """Repositories are validated as on creation.""" + # Arrange + automation = await _seed_prompt_preset_automation( + async_session, + preset_store, + "Same prompt", + preset_metadata={"preset_type": "prompt", "prompt": "Same prompt"}, + ) + + # Act — a path with spaces is neither owner/repo nor a git URL. + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", + json={"repos": [{"url": "org/Web Apps/repo", "provider": "github"}]}, + ) + + # Assert + assert response.status_code == 422 + preset_store.write_stream.assert_not_called() + + async def test_update_repos_fails_when_tarball_cannot_be_rebuilt( + self, async_client, async_session, preset_store + ): + """Repositories are not recorded unless the tarball will clone them.""" + # Arrange — the current tarball object is confirmed absent. + automation = await _seed_prompt_preset_automation( + async_session, + preset_store, + "Same prompt", + preset_metadata={"preset_type": "prompt", "prompt": "Same prompt"}, + ) + original_tarball_path = automation.tarball_path + preset_store.read = MagicMock(side_effect=ObjectNotFoundError("File not found")) + + # Act + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", + json={"repos": [{"url": "owner/repo", "provider": "github"}]}, + ) + + # Assert — nothing changed. + assert response.status_code == 422 + await async_session.refresh(automation) + assert automation.tarball_path == original_tarball_path + assert automation.preset_metadata == { + "preset_type": "prompt", + "prompt": "Same prompt", + } + + async def test_update_repos_as_non_creator_returns_403( + self, async_client, async_session + ): + """Changing repositories edits the definition, so only the creator may.""" + # Arrange — a teammate's preset in the same organization. + automation = Automation( + user_id=OTHER_USER_ID, + org_id=TEST_ORG_ID, + name="Teammate Preset", + trigger={"type": "cron", "schedule": "0 9 * * *", "timezone": "UTC"}, + tarball_path="s3://bucket/code.tar.gz", + entrypoint=".venv/bin/python main.py", + preset_metadata={"preset_type": "prompt", "prompt": "hi"}, + ) + async_session.add(automation) + await async_session.commit() + + # Act + response = await async_client.patch( + f"/api/automation/v1/{automation.id}", json={"repos": []} + ) + + # Assert + assert response.status_code == 403 + async def test_update_name_does_not_regenerate_preset_tarball( self, async_client, async_session, preset_store ):