diff --git a/docs/session-isolation.md b/docs/session-isolation.md index 2ae17fdf..b1584e3c 100644 --- a/docs/session-isolation.md +++ b/docs/session-isolation.md @@ -62,6 +62,25 @@ the caller's source remains an explicit operation. The Workflow CLI accepts `--isolation shared` or `--isolation worktree`. +## Session snapshots + +`opencollab.bootstrap.snapshot_session(session)` creates independent session +state and a forked environment under the [snapshot collaborator requirements](migrations/0.5.0.md). +The source and its successful snapshots share the model transport. When OC +created that transport, each session holds an ownership reference and +`await session.aclose()` releases its reference. The final owner closes the +transport once. A snapshot can continue model requests after its source or +sibling closes. A model injected through `build_session(llm=...)` remains owned +by its caller, who closes it after all sessions finish. + +The snapshot acquires its model reference after its state and runtime +collaborators have copied successfully. Failed construction cleans up its +forked environment. Shared model ownership retains a reference count, allowing +a closed source session to be reclaimed while its snapshots continue. Creating +a new snapshot from a session that has released its model ownership raises +`SnapshotSessionError`. Each session's environment retains its own cleanup +operation. + ## Custom environment providers Pass `environment_providers={"custom": factory}` to `OpenCollab`, then select diff --git a/docs/session-isolation.zh-CN.md b/docs/session-isolation.zh-CN.md index 774369f6..e573dc4c 100644 --- a/docs/session-isolation.zh-CN.md +++ b/docs/session-isolation.zh-CN.md @@ -46,6 +46,20 @@ topology: Workflow CLI 支持 `--isolation shared` 和 `--isolation worktree`。 +## Session 快照 + +`opencollab.bootstrap.snapshot_session(session)` 按照[快照协作者要求](migrations/0.5.0.md), +创建独立的会话状态和分叉环境。源会话与成功创建的快照共享模型传输。 +OC 创建的传输由每个会话分别持有一份所有权引用, +`await session.aclose()` 释放该会话的引用,最后一个拥有者关闭传输一次。 +源会话或兄弟快照关闭后,存活快照仍可继续请求模型。 +通过 `build_session(llm=...)` 注入的模型由调用者拥有,调用者在所有会话结束后关闭它。 + +快照在状态和运行时协作者全部复制成功后取得模型引用,构建失败会清理已分叉的环境。 +共享模型的所有权通过引用计数保存,源会话关闭后可以回收,存活快照继续运行。 +从已释放模型所有权的会话创建新快照会抛出 `SnapshotSessionError`。 +各会话的环境分别执行自己的清理操作。 + ## 自定义环境提供器 向 `OpenCollab` 传入 `environment_providers={"custom": factory}`,随后使用 `isolation="custom"`。每个工厂通过关键字参数接收 `workspace`、`base_environment`、`owned_save_dir`、`enable_adoption` 和 `owned_output_paths`。 diff --git a/docs/weave.md b/docs/weave.md index 0e6a6e92..963741eb 100644 --- a/docs/weave.md +++ b/docs/weave.md @@ -73,6 +73,13 @@ test output. An executable probe uses a `command` and its `expected_output`, as shown above. Include assertions in the probe so the expected output follows the behavior being checked. +Weave adds `-rA` to recognized pytest invocations to retain the executed test +nodes, including ordinary `python -m pytest test_result.py -q` checks. The +verification report keeps the supplied `command` and records `executed_command` +when reporting flags were added. Shell combinations outside the supported test +parser require an application verifier or an assertion script with +`expected_output`. + ## Run through the public SDK This script uses the installed package, creates ordinary files, and verifies @@ -190,6 +197,7 @@ rounds. `max_repair_rounds` and `stagnant_round_limit` bound repair attempts. An adapter's `save_state(data)` hook receives independent JSON snapshots for storage. Supply a saved snapshot as the JSON `state` input or through `WeaveState(saved_snapshot)` with `run_weave`. Reuse the saved `run_id` -when calling the SDK. Generated groups continue at their checks, checked groups +when calling the SDK or pass it as `workflow run weave --run-id RUN_ID` through +the CLI. Generated groups continue at their checks, checked groups retain their generation results, and consumed repair rounds remain counted. The saved group fields must agree with the supplied groups. diff --git a/opencollab/adapters/_docker_snapshot.py b/opencollab/adapters/_docker_snapshot.py index e08bef6f..f233dc15 100644 --- a/opencollab/adapters/_docker_snapshot.py +++ b/opencollab/adapters/_docker_snapshot.py @@ -5,6 +5,7 @@ import posixpath import shlex import shutil +import stat import tempfile from contextlib import asynccontextmanager from pathlib import Path @@ -14,8 +15,10 @@ from opencollab.adapters._env_docker import DockerEnvironment from opencollab.adapters._env_local import LocalEnvironment from opencollab.adapters._env_vm import VMEnvironment +from opencollab.adapters._env_worktree import WorktreeEnvironment from opencollab.adapters._git_patch_bytes import _complete, _patch_temporary from opencollab.adapters._team_candidate_environment import _TeamCandidateEnvironment +from opencollab.adapters.safe_anchored_files import open_regular_read_at from opencollab.application.async_timeout import await_owned_operation @@ -40,6 +43,38 @@ async def _copy_to_host(environment: Any, source: str, target: Path, *, director raise OSError("Docker source workspace copy failed") +async def _copy_artifact_to_host(environment: Any, source: str, target: Path) -> None: + relative = ( + posixpath.relpath(source, environment.workspace) if posixpath.isabs(source) else posixpath.normpath(source) + ) + if relative == ".." or relative.startswith("../"): + raise PermissionError("artifact path escapes workspace") + source = posixpath.join(environment.workspace, relative) + delegate = environment._delegate if isinstance(environment, _TeamCandidateEnvironment) else environment + if isinstance(delegate, WorktreeEnvironment): + delegate._ensure_active() + if delegate._local_env is None: + await delegate.setup() + delegate = delegate._local_env + if isinstance(delegate, LocalEnvironment): + def copy() -> None: + with open_regular_read_at( + delegate._root_fd(), delegate.workspace, delegate._relative_path(source), + ) as handle, target.open("wb") as output: + shutil.copyfileobj(handle, output) + + await delegate._run_file_operation(copy) + return + if isinstance(delegate, DockerEnvironment): + source = await delegate._contained_file_path(source) + if isinstance(delegate, (DockerEnvironment, VMEnvironment)): + await _copy_to_host(delegate, source, target) + if not stat.S_ISREG(target.lstat().st_mode): + raise OSError("artifact transfer requires a regular file") + return + target.write_text(await environment.read_file(source), encoding="utf-8") + + def _remove_marker(path: Path) -> None: marker = path / ".git" if marker.is_dir() and not marker.is_symlink(): diff --git a/opencollab/adapters/_env_worktree.py b/opencollab/adapters/_env_worktree.py index 39161643..3c8905f5 100644 --- a/opencollab/adapters/_env_worktree.py +++ b/opencollab/adapters/_env_worktree.py @@ -679,12 +679,12 @@ async def _ensure_directory_copy_changes_exported(self) -> None: "call get_diff() and deliver its artifact first" ) - async def exec_cmd(self, cmd: str, timeout: float = 120.0) -> ExecResult: + async def exec_cmd(self, cmd: str, timeout: float = 120.0, *, input_bytes: bytes | None = None) -> ExecResult: self._ensure_active() if self._local_env is None: await self.setup() assert self._local_env is not None - return await self._local_env.exec_cmd(cmd, timeout) + return await self._local_env.exec_cmd(cmd, timeout, input_bytes=input_bytes) async def read_file(self, path: str) -> str: self._ensure_active() diff --git a/opencollab/adapters/cli/main.py b/opencollab/adapters/cli/main.py index c4570414..0ac78e83 100644 --- a/opencollab/adapters/cli/main.py +++ b/opencollab/adapters/cli/main.py @@ -13,6 +13,7 @@ import sys import uuid from collections.abc import Awaitable +from contextvars import ContextVar from typing import Any, Optional import typer @@ -366,10 +367,18 @@ async def _run( # ``--yolo`` only auto-approves risky commands; a human is still present, so # the ask-user tool stays interactive — its question takes the shared input # line ahead of whatever the user was typing. + turn_cancel: ContextVar[asyncio.Event | None] = ContextVar("cli_turn_cancel", default=None) + + async def read_question(text: str) -> str: + cancel = turn_cancel.get() + if cancel is not None and cancel.is_set(): + raise EOFError + return await prompt.ask(text) + permission_policy = None if not yolo: - permission_policy = TuiPermissionPolicy(read_line=prompt.ask) - ask_policy = TuiAskUserPolicy(read_line=prompt.ask) + permission_policy = TuiPermissionPolicy(read_line=read_question) + ask_policy = TuiAskUserPolicy(read_line=read_question) event_sink = TuiEventSink(tui) events_file = os.environ.get("OPENCOLLAB_EVENTS_FILE") @@ -418,6 +427,7 @@ async def turn(line: str, target_aid: int, cancel: asyncio.Event) -> None: tui.record_user_message(target_aid, line) tui.start_turn(target_aid) failure: SchedulerTurnError | None = None + cancel_token = turn_cancel.set(cancel) try: await scheduler.run_turn(target_aid, line, cancel_event=cancel) except SchedulerTurnError as exc: @@ -425,6 +435,7 @@ async def turn(line: str, target_aid: int, cancel: asyncio.Event) -> None: except KeyboardInterrupt: pass finally: + turn_cancel.reset(cancel_token) # Flush the answering agent's unprinted tail when focus moved to a teammate. # Switching focus would reprint rows already in scrollback. tui.settle_turn( diff --git a/opencollab/adapters/cli/workflow.py b/opencollab/adapters/cli/workflow.py index 2734e15e..d53a3711 100644 --- a/opencollab/adapters/cli/workflow.py +++ b/opencollab/adapters/cli/workflow.py @@ -86,6 +86,7 @@ def run_cmd( api_key: Optional[str] = typer.Option(None, "--api-key", help="API key (default from config)"), base_url: Optional[str] = typer.Option(None, "--base-url", help="API base URL (default from config)"), workspace: str = typer.Option(".", "--workspace", "-w", help="Working directory"), + run_id: Optional[str] = typer.Option(None, "--run-id", help="Original run identity for workflow continuation"), budget: Optional[int] = typer.Option( None, "--budget", @@ -177,6 +178,7 @@ def run_cmd( agent_profile=agent_profile, **({"isolation": isolation} if isolation is not None else {}), **({"cleanup_timeout": cleanup_timeout} if cleanup_timeout is not None else {}), + **({"run_id": run_id} if run_id is not None else {}), save_dir=save_dir, trace=trace, return_details=True, diff --git a/opencollab/adapters/git_patch.py b/opencollab/adapters/git_patch.py index d55e3665..081402f5 100644 --- a/opencollab/adapters/git_patch.py +++ b/opencollab/adapters/git_patch.py @@ -36,16 +36,18 @@ def _git_command_and_config_guard(*, allow_core_worktree: bool = False) -> tuple "for config_scope in $config_scopes; do " f"unsafe_config=$({git_command} config \"$config_scope\" --includes --name-only --get-regexp " "'^(diff\\..*\\.(command|textconv)|filter\\..*|" - f"core\\.(excludesfile|sparsecheckout|sparsecheckoutcone{worktree_setting}))$' " + f"core\\.(excludesfile{worktree_setting}))$' " "2>/dev/null); " 'config_rc=$?; if [ "$config_rc" -eq 0 ]; then ' "echo \"unsafe repository Git configuration: $unsafe_config\" >&2; exit 125; " 'elif [ "$config_rc" -ne 1 ]; then exit "$config_rc"; fi; ' - f'fsmonitor=$({git_command} config "$config_scope" --includes --type=bool ' - "--get core.fsmonitor 2>/dev/null); fsmonitor_rc=$?; " - 'if [ "$fsmonitor_rc" -eq 0 ] && [ "$fsmonitor" = false ]; then :; ' - 'elif [ "$fsmonitor_rc" -ne 1 ]; then ' - "echo 'unsafe repository Git configuration: core.fsmonitor' >&2; exit 125; fi; " + "for boolean_key in core.fsmonitor core.sparsecheckout core.sparsecheckoutcone; do " + f'config_value=$({git_command} config "$config_scope" --includes --type=bool ' + '--get "$boolean_key" 2>/dev/null); config_rc=$?; ' + 'if [ "$config_rc" -eq 0 ] && [ "$config_value" = false ]; then :; ' + 'elif [ "$config_rc" -ne 1 ]; then ' + 'echo "unsafe repository Git configuration: $boolean_key" >&2; exit 125; fi; ' + "done; " "done; " f'info_attributes=$({git_command} rev-parse --git-path info/attributes) || exit 125; ' 'if [ -L "$info_attributes" ] || { [ -e "$info_attributes" ] && [ ! -f "$info_attributes" ]; }; then ' @@ -164,13 +166,11 @@ def guarded_staged_diff_command( ``core.attributesFile`` and ``diff.ignoreSubmodules`` are pinned by an explicit ``-c``, which outranks any repository-local value, so a repository that marks a file undiffable or hides a gitlink change cannot do either - here. ``core.fsmonitor`` is judged by its value: a repository that turns a - monitor off is hardening itself and is allowed through, while one that turns - a monitor on is refused, because a monitor that lies about what changed can - only be trusted or refused, never overridden. Everything else that could - bend the reading -- a redirected working tree, a sparse checkout, an - excludes file, and the clean/smudge filters and external diff drivers a - tracked ``.gitattributes`` can still reach -- is refused outright. + here. ``core.fsmonitor``, ``core.sparsecheckout``, and + ``core.sparsecheckoutcone`` accept Git boolean false values or an absent + setting. Enabled or malformed values are refused. Redirected working trees, + excludes files, clean/smudge filters, and external diff drivers reachable + through tracked ``.gitattributes`` are refused outright. A refusal exits 125 rather than reporting a patch this command cannot vouch for. diff --git a/opencollab/adapters/llm/anthropic_provider.py b/opencollab/adapters/llm/anthropic_provider.py index 8453bbc5..6a1fc61e 100644 --- a/opencollab/adapters/llm/anthropic_provider.py +++ b/opencollab/adapters/llm/anthropic_provider.py @@ -548,7 +548,7 @@ def convert_to_anthropic_messages(messages: list[dict]) -> tuple[list[str], list Returns (system_parts, anthropic_messages). Key conversions: - - role="system" → extracted to system_parts (Anthropic uses top-level system param) + - role="system"/"developer" → extracted to Anthropic's top-level system param - role="assistant" with tool_calls → assistant with tool_use content blocks - role="tool" → role="user" with tool_result content blocks (merged if consecutive) """ @@ -558,7 +558,7 @@ def convert_to_anthropic_messages(messages: list[dict]) -> tuple[list[str], list for message_index, message in enumerate(messages): role = message.get("role", "") - if role == "system": + if role in {"system", "developer"}: content = message.get("content", "") blocks = _normalize_openai_content( content, diff --git a/opencollab/adapters/safe_anchored_files.py b/opencollab/adapters/safe_anchored_files.py index 6625bfeb..aeff6f22 100644 --- a/opencollab/adapters/safe_anchored_files.py +++ b/opencollab/adapters/safe_anchored_files.py @@ -10,7 +10,8 @@ import os import stat import uuid -from collections.abc import Callable +from collections.abc import Callable, Iterator +from contextlib import contextmanager from typing import BinaryIO from opencollab.adapters._env_base import TextFileRange @@ -116,15 +117,13 @@ def open_parent_beneath( os.close(current_fd) raise -def read_regular_bytes_at( +@contextmanager +def open_regular_read_at( root_fd: int, root_path: str | os.PathLike[str], path: str | os.PathLike[str], - *, - max_bytes: int, -) -> bytes: - """Read a bounded regular file beneath a pinned directory descriptor.""" - limit = _require_limit(max_bytes) +) -> Iterator[BinaryIO]: + """Open a regular file beneath a pinned directory without following symlinks.""" parent_fd, name = open_parent_beneath( root_fd, root_path, @@ -141,6 +140,28 @@ def read_regular_bytes_at( opened = os.fstat(fd) if not stat.S_ISREG(opened.st_mode): raise OSError(f"read target is not a regular file: {path}") + handle = os.fdopen(fd, "rb") + fd = -1 + with handle: + yield handle + finally: + if fd >= 0: + os.close(fd) + os.close(parent_fd) + + +def read_regular_bytes_at( + root_fd: int, + root_path: str | os.PathLike[str], + path: str | os.PathLike[str], + *, + max_bytes: int, +) -> bytes: + """Read a bounded regular file beneath a pinned directory descriptor.""" + limit = _require_limit(max_bytes) + with open_regular_read_at(root_fd, root_path, path) as handle: + fd = handle.fileno() + opened = os.fstat(fd) if opened.st_size > limit: raise ValueError(f"read target exceeds {limit}-byte limit: {path}") chunks: list[bytes] = [] @@ -155,10 +176,6 @@ def read_regular_bytes_at( if len(payload) > limit: raise ValueError(f"read target exceeds {limit}-byte limit: {path}") return payload - finally: - if fd >= 0: - os.close(fd) - os.close(parent_fd) def read_regular_text_range_at( root_fd: int, diff --git a/opencollab/adapters/storage.py b/opencollab/adapters/storage.py index b9da8a7d..e2f60f41 100644 --- a/opencollab/adapters/storage.py +++ b/opencollab/adapters/storage.py @@ -141,7 +141,7 @@ def write(self, text: str) -> int: return len(text) class SessionStore: - allowed_roles = {"system", "user", "assistant", "tool"} + allowed_roles = {"system", "developer", "user", "assistant", "tool"} def has_snapshot(self, path: str) -> bool: """Return whether a base or recoverable journal sidecar is present.""" @@ -296,7 +296,7 @@ def _validate_messages(self, messages: list[dict[str, Any]]) -> None: role = msg.get("role") if role not in self.allowed_roles: raise ValueError(f"Invalid message role at position {lineno}: {role}") - if role in {"system", "user"}: + if role in {"system", "developer", "user"}: self._validate_content(msg, lineno) elif role == "assistant": tool_calls = msg.get("tool_calls") diff --git a/opencollab/adapters/tools/evidence/bash_evidence.py b/opencollab/adapters/tools/evidence/bash_evidence.py index f095a69f..8c62d2de 100644 --- a/opencollab/adapters/tools/evidence/bash_evidence.py +++ b/opencollab/adapters/tools/evidence/bash_evidence.py @@ -4,18 +4,20 @@ import copy import os.path import posixpath +import shlex from typing import Any from opencollab.application.ports import ToolPort as Tool from ._test_results import ( _is_pytest_runner, + _parts_invoke_pytest, _passed_tests, _target_has_pass_proof, _verification_targets_overlap, has_pass_evidence, ) -from .command_evidence import parse_test_command, relative_target +from .command_evidence import _strip_command_padding, parse_test_command, relative_target def _pytest_target(target: str, workspace: str | None, local_filesystem: bool) -> str: @@ -101,6 +103,30 @@ def __init__(self, delegate: Tool): def __getattr__(self, name: str) -> Any: return getattr(self._delegate, name) + def with_pass_reporting(self, command: str) -> str: + command = _strip_command_padding(command) + spec = parse_test_command(command, None) + if spec is None or not spec.provable or not _is_pytest_runner(spec.runner): + return command + lexer = shlex.shlex(command, posix=True) + lexer.whitespace_split = True + lexer.commenters = "" + runner_seen = False + while True: + start = lexer.instream.tell() + token = lexer.get_token() + if token is None: + return command + " -rA" + if runner_seen and token == "--": + return command[:start] + "-rA " + command[start:] + if token == "&&": + runner_seen = False + if token.rsplit("/", 1)[-1] == "pytest": + runner_seen = True + + def invokes_pytest(self, parts: list[str]) -> bool: + return _parts_invoke_pytest(parts) + def fork_verification_scope(self, scope: dict[object, Any]) -> BashEvidence: """Share one fresh observer among this call's Bash and edit tools.""" if self not in scope: diff --git a/opencollab/adapters/tools/evidence/command_evidence.py b/opencollab/adapters/tools/evidence/command_evidence.py index 9cbbfb91..46c24c79 100644 --- a/opencollab/adapters/tools/evidence/command_evidence.py +++ b/opencollab/adapters/tools/evidence/command_evidence.py @@ -41,9 +41,22 @@ def _targets(arguments: list[str], value_options: set[str]) -> tuple[str, ...] | index += 1 return tuple(targets) or (".",) +def _strip_command_padding(command: str) -> str: + end = len(command.rstrip()) + escaped = end + while escaped and command[escaped - 1] == "\\": + escaped -= 1 + # The first escaped whitespace character belongs to the final argument. + if end < len(command) and (end - escaped) % 2: + end += 1 + return command[:end].lstrip() + def parse_test_command(command: str, workspace: str | None) -> TestCommand | None: """Recognize direct pytest, Go and Django invocations, never shell output replay.""" - if not isinstance(command, str) or any(char in command for char in "\n\r;|<>`"): + if not isinstance(command, str): + return None + command = _strip_command_padding(command) + if any(char in command for char in "\n\r;|<>`"): return None try: parts = shlex.split(command) diff --git a/opencollab/application/compaction_summary.py b/opencollab/application/compaction_summary.py index 088b2ed4..a3913d03 100644 --- a/opencollab/application/compaction_summary.py +++ b/opencollab/application/compaction_summary.py @@ -153,6 +153,8 @@ async def asummarize(self, segment: list[dict[str, Any]]) -> str: async def _complete(self, request: list[dict[str, Any]]) -> str: response = await self._acomplete(request) + if getattr(response, "finish_reason", None) in {"length", "max_tokens", "model_context_window_exceeded"}: + raise ValueError("The model returned a truncated conversation summary") return format_compact_summary(getattr(response, "content", None) or "") def _fits(self, request: list[dict[str, Any]], *, allow_budget_stop: bool = False) -> bool: diff --git a/opencollab/application/session.py b/opencollab/application/session.py index 190c1a0f..74e2a51c 100644 --- a/opencollab/application/session.py +++ b/opencollab/application/session.py @@ -51,6 +51,10 @@ class SessionBusyError(RuntimeError): class _InheritedLimit(Enum): RUNTIME = "runtime" +@dataclass +class _SharedLLMOwnership: + owners: int = 1 + @dataclass class SessionRuntime: """Pre-built collaborators a ``Session`` facade keeps as attributes. @@ -137,6 +141,7 @@ def __init__( self._auto_save_subscriber = runtime.auto_save_subscriber self.runner.late_provider_usage_checkpoint = self.enqueue_auto_save self._owns_llm = runtime.owns_llm + self._shared_llm_ownership: _SharedLLMOwnership | None = None self._llm_closed = False self._llm_close_error: BaseException | None = None self._terminal_checkpoint_error: Exception | None = None @@ -257,7 +262,7 @@ def persistence_errors(self) -> tuple[Exception, ...]: return tuple(errors) async def aclose(self) -> None: - """Close the model transport when this session created and owns it.""" + """Release the owned model transport after its last session snapshot closes.""" await self._close_owned_runtimes() if self._llm_close_error is not None: raise self._llm_close_error @@ -265,6 +270,11 @@ async def aclose(self) -> None: self.runner.finish_run_control(lambda: self.pending_cleanup_tasks) return self._llm_closed = True + if self._shared_llm_ownership is not None: + self._shared_llm_ownership.owners -= 1 + if self._shared_llm_ownership.owners: + self.runner.finish_run_control(lambda: self.pending_cleanup_tasks) + return close = getattr(self._llm, "close", None) if not callable(close): close = getattr(self._llm, "aclose", None) @@ -279,6 +289,14 @@ async def aclose(self) -> None: raise self.runner.finish_run_control(lambda: self.pending_cleanup_tasks) + def _share_llm_ownership(self, snapshot: Session) -> None: + if self._owns_llm: + if self._shared_llm_ownership is None: + self._shared_llm_ownership = _SharedLLMOwnership() + self._shared_llm_ownership.owners += 1 + snapshot._shared_llm_ownership = self._shared_llm_ownership + snapshot._owns_llm = True + def own_runtime( self, close: Callable[[], Awaitable[None]], *, reserve_tokens: Callable[[], int] | None = None, ) -> None: diff --git a/opencollab/application/workflow.py b/opencollab/application/workflow.py index d7ff7943..491f8a27 100644 --- a/opencollab/application/workflow.py +++ b/opencollab/application/workflow.py @@ -708,7 +708,7 @@ def _internal_commit_deadline(self) -> float: return time.monotonic() + timeout def _track_session(self, session: Any) -> None: - self._sessions.append(session) + self.budget._track_session(session) lease = self._active_budget_lease.get() if lease is not None: lease.sessions.append(session) diff --git a/opencollab/application/workflow_budget.py b/opencollab/application/workflow_budget.py index 1aed9a90..a5912a1d 100644 --- a/opencollab/application/workflow_budget.py +++ b/opencollab/application/workflow_budget.py @@ -54,16 +54,24 @@ def total(self) -> int | None: def spent(self) -> int: return sum(int(getattr(s, "used_tokens", 0)) for s in self._sessions) + def _track_session(self, session: Any) -> None: + self._sessions.append(session) + if self._parent_allocation is not None: + parent, allocation = self._parent_allocation + allocation.sessions.append(session) + parent._track_session(session) + def remaining(self) -> float: if self._total is None: return float("inf") total = self._total + spent = self.spent() if self._parent_allocation is not None: parent, allocation = self._parent_allocation # A sibling can overspend while this workflow waits for an agent slot. - total = min(total, parent.remaining() + allocation.remaining()) + total = min(total, spent + parent.remaining() + allocation.remaining()) reserved_unspent = sum(lease.remaining() for lease in self._leases) - return total - self.spent() - reserved_unspent + return total - spent - reserved_unspent def reserve(self, lease: _BudgetLease) -> None: self._leases.append(lease) diff --git a/opencollab/application/workflow_candidates.py b/opencollab/application/workflow_candidates.py index 5c49ca7d..d54b5f3e 100644 --- a/opencollab/application/workflow_candidates.py +++ b/opencollab/application/workflow_candidates.py @@ -610,7 +610,6 @@ async def run() -> CandidateRun: else: failure = exc finally: - self._sessions.extend(child.sessions) for agent_failure in child.agent_failures: self._agent_failures.append({ **agent_failure, diff --git a/opencollab/application/workflow_collections.py b/opencollab/application/workflow_collections.py index 65853e57..d11c746c 100644 --- a/opencollab/application/workflow_collections.py +++ b/opencollab/application/workflow_collections.py @@ -154,7 +154,7 @@ async def _bounded_collection( planned_budget = max(1, available // size) if available else 0 results: list[Any] = [None] * size next_index = iter(range(size)) - terminal: list[BaseException] = [] + errors: dict[int, BaseException] = {} async def execute(index: int) -> None: budget_token = self._active_collection_budget.set(planned_budget) @@ -165,13 +165,12 @@ async def execute(index: int) -> None: except asyncio.CancelledError: raise except BaseException as exc: - results[index] = exc - terminal.append(exc) + errors[index] = exc finally: self._active_collection_budget.reset(budget_token) async def worker() -> None: - while not terminal: + while not errors: try: index = next(next_index) except StopIteration: @@ -242,12 +241,11 @@ async def allocate_workers() -> None: ) raise - for result in results: - if isinstance(result, WorkflowBudgetExceeded): - raise result - for result in results: - if isinstance(result, BaseException): - raise result + for index in sorted(errors): + if isinstance(errors[index], WorkflowBudgetExceeded): + raise errors[index] + if errors: + raise errors[min(errors)] return results async def parallel(self, thunks: Sequence[Thunk]) -> list[Any]: diff --git a/opencollab/bootstrap/authored_workflow.py b/opencollab/bootstrap/authored_workflow.py index 725bfc45..b6cb00da 100644 --- a/opencollab/bootstrap/authored_workflow.py +++ b/opencollab/bootstrap/authored_workflow.py @@ -16,6 +16,8 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, get_args +from opencollab.adapters._docker_snapshot import _copy_artifact_to_host +from opencollab.adapters._env_process import ProcessCleanupError from opencollab.adapters.session_environments import SessionEnvironmentPool from opencollab.adapters.tools._paths import checked_path from opencollab.adapters.tools.base import Tool @@ -101,6 +103,7 @@ def __init__( save_dir: str | None, *, allow_unisolated_shell: bool = False, environment_pool_factory: Callable[..., Any] | None = None, environment_catalog: Callable[[], dict[str, Any]] | None = None, + describe_environment: Callable[..., dict[str, Any]] | None = None, ) -> None: self._get_session = get_session self._config = config @@ -108,12 +111,16 @@ def __init__( self._allow_unisolated_shell = allow_unisolated_shell self._environment_pool_factory = environment_pool_factory self._environment_catalog = environment_catalog + self._describe_environment = describe_environment self.parameters = deepcopy(type(self).parameters) self._children: dict[str, _Child] = {} self._registered = False self._launch_lock = asyncio.Lock() def _catalog(self) -> dict[str, Any]: + if self._describe_environment is not None: + parent = self._get_session() + return self._describe_environment(parent.env, role=parent.agent.name) if self._environment_catalog is not None: return self._environment_catalog() return {"default_isolation": "shared", @@ -320,15 +327,16 @@ def _account_usage(self, child: _Child, tokens: Any) -> None: child.tokens = tokens async def _execute(self, child: _Child, command: str, request_bytes: bytes) -> None: - execution = asyncio.create_task(child.environment.exec_cmd( - command, timeout=math.inf, input_bytes=request_bytes, - )) - child.execution = execution - if child.stop_requested: - execution.cancel() + execution = None logs: dict[str, Any] = {} status = "running" try: + execution = asyncio.create_task(child.environment.exec_cmd( + command, timeout=math.inf, input_bytes=request_bytes, + )) + child.execution = execution + if child.stop_requested: + execution.cancel() while not execution.done(): await asyncio.wait({execution}, timeout=0.1) assert child.bridge is not None @@ -340,7 +348,12 @@ async def _execute(self, child: _Child, command: str, request_bytes: bytes) -> N logs = {"returncode": outcome.returncode, "stdout": outcome.stdout, "stderr": outcome.stderr} if outcome.returncode: raise RuntimeError(f"Workflow runner exited {outcome.returncode}: {outcome.stderr[-2000:]}") - child.result = json.loads(await child.environment.read_file(posixpath.join(child.artifacts, "result.json"))) + with tempfile.TemporaryDirectory(prefix="opencollab-child-result-") as temporary: + result_path = Path(temporary) / "result.json" + await _copy_artifact_to_host( + child.environment, posixpath.join(child.artifacts, "result.json"), result_path, + ) + child.result = json.loads(result_path.read_text(encoding="utf-8")) self._account_usage(child, child.result["tokens"]) status = child.result["status"] except asyncio.CancelledError: @@ -350,14 +363,16 @@ async def _execute(self, child: _Child, command: str, request_bytes: bytes) -> N status = "failed" child.result = {"status": "failed", "output": None, "reason": f"{type(exc).__name__}: {exc}"} finally: - if not execution.done(): - execution.cancel() - settled = await asyncio.gather(execution, return_exceptions=True) - if isinstance(settled[0], Exception): - status = "failed" - child.cleanup_error = settled[0] - child.result = {"status": "failed", "output": None, - "reason": f"child process cleanup failed: {settled[0]}"} + if execution is not None: + cancelling = not execution.done() + if cancelling: + execution.cancel() + settled = await asyncio.gather(execution, return_exceptions=True) + if isinstance(settled[0], ProcessCleanupError) or (cancelling and isinstance(settled[0], Exception)): + status = "failed" + child.cleanup_error = settled[0] + child.result = {"status": "failed", "output": None, + "reason": f"child process cleanup failed: {settled[0]}"} try: assert child.bridge is not None await await_owned_operation(child.bridge.aclose(), propagate_cancellation=False) @@ -388,10 +403,11 @@ async def _execute(self, child: _Child, command: str, request_bytes: bytes) -> N ): raise ValueError("invalid child artifact name") try: - content = await child.environment.read_file(posixpath.join(child.artifacts, name)) + await _copy_artifact_to_host( + child.environment, posixpath.join(child.artifacts, name), directory / name, + ) except FileNotFoundError: continue - (directory / name).write_text(content, encoding="utf-8") child.saved_artifacts = str(directory) (directory / "result.json").write_text(json.dumps(self._snapshot(child, include_result=True)), encoding="utf-8") diff --git a/opencollab/bootstrap/session_factory.py b/opencollab/bootstrap/session_factory.py index 66e02f84..7d24bf3b 100644 --- a/opencollab/bootstrap/session_factory.py +++ b/opencollab/bootstrap/session_factory.py @@ -12,6 +12,7 @@ import os import re import uuid +from dataclasses import replace from datetime import datetime from pathlib import Path from typing import Any, Callable @@ -22,10 +23,13 @@ from opencollab.adapters.llm.types import model_capabilities from opencollab.adapters.repo_map import build_repo_map from opencollab.adapters.safe_files import ensure_directory_no_symlinks +from opencollab.adapters.safety import SandboxInterceptor +from opencollab.adapters.single2_safety import Single2SafetyPolicy from opencollab.adapters.storage import SessionStore from opencollab.adapters.tools.base import Tool from opencollab.adapters.trace import Tracer from opencollab.application.compaction_summary import run_coro_blocking +from opencollab.application.event_bus import EventBus from opencollab.application.exception_notes import add_exception_note from opencollab.application.ports import ( AskUserPort, @@ -309,20 +313,54 @@ def snapshot_session( silently sharing a workspace would make the resulting sessions unsafe to run concurrently. """ + from opencollab.bootstrap.authored_workflow import AuthoredWorkflowTool + from opencollab.bootstrap.team_workflow import TeamWorkflowTool + + if session._llm_closed: + raise SnapshotSessionError("cannot snapshot a session with closed model ownership") + session_tools = (*session.agent.tools, *session.runner._initial_agent_tools) + # Exclude callable owners from deepcopy. Python 3.14 functions bypass memo lookup. + owner_bindings = { + id(tool._get_session): lambda: new + for tool in session_tools + if isinstance(tool, (AuthoredWorkflowTool, TeamWorkflowTool)) + } + for tool in session_tools: + if isinstance(tool, AuthoredWorkflowTool): + # Keep configured provider owners outside the session's independently copied state. + for callback in (tool._environment_pool_factory, tool._environment_catalog, tool._describe_environment): + if callback is not None: + owner_bindings[id(callback)] = callback + for tool in session_tools: + if isinstance(tool, (AuthoredWorkflowTool, TeamWorkflowTool)) and id(tool._config) not in owner_bindings: + config = replace(tool._config, tracer=None, event_bus=EventBus()) + owner_bindings[id(tool._config)] = _clone_snapshot_component( + config, label="workflow configuration", memo=owner_bindings, + ) agent, initial_tools, initial_tool_choice = _clone_snapshot_component( (session.agent, session.runner._initial_agent_tools, session.runner._initial_agent_tool_choice), label="agent", + memo=owner_bindings, ) + for tool in (*agent.tools, *initial_tools): + if isinstance(tool, (AuthoredWorkflowTool, TeamWorkflowTool)): + tool._get_session = lambda: new + if isinstance(tool, AuthoredWorkflowTool): + tool._registered = False permission_policy = _clone_snapshot_component(session.permission_policy, label="permission policy") ask_policy = _clone_snapshot_component(session.tool_execution.ask_policy, label="ask policy") safety_policy = _clone_snapshot_component(session.tool_execution.safety_policy, label="safety policy") state = _clone_snapshot_component(session.state, label="session state") environment = _fork_snapshot_environment(session.env) try: + _rebind_snapshot_safety_policy(safety_policy, session.env.workspace, environment.workspace) + control = session.runner._run_control + if control is not None: + control = replace(control, initial_soft_budget_tokens=session.max_budget_tokens, on_event=None) new = build_session( agent=agent, env=environment, - max_budget_tokens=session.max_budget_tokens, + max_budget_tokens=session.runner.hard_budget_tokens, max_steps=session.max_steps, event_sink=event_sink, permission_policy=permission_policy, @@ -332,7 +370,12 @@ def snapshot_session( llm_timeout=session.runner._per_call_timeout, aid=session.state.aid, run_id=session.run_id, + run_control=control, ) + new.max_budget_tokens = session.max_budget_tokens + # Soft-budget changes preserve the source's already-effective reserve. + new.runner._enforcement_strength = session.runner._enforcement_strength + new.runner._commit_reserve = session.runner._commit_reserve if state.phase not in { SessionPhase.IDLE, SessionPhase.DONE, SessionPhase.STOPPED, SessionPhase.ERROR, } or new._open_tool_call_ids(state): @@ -363,6 +406,7 @@ def snapshot_session( if shaper is not None and shaper is session.runner.shaper: raise SnapshotSessionError("cannot clone context shaper for an independent snapshot") new.runner.shaper = shaper + session._share_llm_ownership(new) return new except BaseException as error: try: @@ -376,6 +420,21 @@ def snapshot_session( ) raise +def _rebind_snapshot_safety_policy(policy: Any, source_workspace: str, workspace: str) -> None: + def relocate(path: str) -> str: + source_root = os.path.realpath(source_workspace) + absolute = os.path.realpath(path) + if os.path.commonpath((source_root, absolute)) == source_root: + return os.path.abspath(os.path.join(workspace, os.path.relpath(absolute, source_root))) + return path + + if isinstance(policy, Single2SafetyPolicy): + policy.root = relocate(policy.root) + _rebind_snapshot_safety_policy(policy._policy, source_workspace, workspace) + elif isinstance(policy, SandboxInterceptor): + policy.root = relocate(policy.root) + policy.allowed_roots = [relocate(root) for root in policy.allowed_roots] + def _clone_snapshot_component(value: Any, *, label: str, memo: dict[int, Any] | None = None) -> Any: try: return copy.deepcopy(value, memo) @@ -701,9 +760,7 @@ def _authored_workflow_tool( return AuthoredWorkflowTool( get_session, self._cfg, self._save_dir, environment_pool_factory=self._environment_pool_factory, - environment_catalog=( - lambda: self._environment_catalog(get_session().env, role=get_session().agent.name) - ) if self._environment_catalog is not None else None, + describe_environment=self._environment_catalog, allow_unisolated_shell=self._unisolated_shell_allowed(seated_at_start=seated_at_start), ) diff --git a/opencollab/builtin_workflows/_weave_execution.py b/opencollab/builtin_workflows/_weave_execution.py index 93001ebe..941e0cf5 100644 --- a/opencollab/builtin_workflows/_weave_execution.py +++ b/opencollab/builtin_workflows/_weave_execution.py @@ -71,6 +71,13 @@ async def execute_with_runtime(self, params: dict[str, Any], runtime: Any) -> st captured.environment = _CaptureEnvironment(environment, self.results) if environment is not None else None return await self._delegate.execute_with_runtime(params, captured) + def prepare_command(self, command: str) -> str: + return self._delegate.with_pass_reporting(command) + + def requires_test_evidence(self, command: str) -> bool: + return any(self._delegate.invokes_pytest(_command_words(shlex.split(segment))) + for segment in _command_segments(command)) + def _command_spec(value: Any) -> tuple[str, str | None]: if isinstance(value, str): return value, None @@ -81,15 +88,69 @@ def _command_spec(value: Any) -> tuple[str, str | None]: raise ValueError("a command probe requires command text and nonempty expected_output") return command, expected +def _command_segments(command: str) -> list[str]: + quote = None + escaped = False + start = 0 + segments = [] + for index, char in enumerate(command): + if index < start: + continue + if escaped: + escaped = False + elif char == "\\" and quote != "'": + escaped = True + elif char == quote: + quote = None + elif quote is None and char in {"'", '"'}: + quote = char + elif quote is None and char in "(){}": + return [] + elif quote is None and char == "#" and (index == 0 or command[index - 1].isspace()): + if any(line.strip() for line in command[index:].splitlines()[1:]): + return [] + command = command[:index] + break + elif quote != "'" and (char == "`" or command[index:index + 2] == "$("): + return [] + elif quote is None and command[index:index + 2] == "&&": + if command[start:index].strip(): + segments.append(command[start:index]) + start = index + 2 + elif quote is None and char in ";|&": + return [] + elif quote is None and char in "\n\r" and command[:index].strip() and command[index:].strip(): + return [] + if command[start:].strip(): + segments.append(command[start:]) + return segments + +def _command_words(tokens: list[str]) -> list[str]: + for index, token in enumerate(tokens): + variable, separator, _ = token.partition("=") + if separator and variable.isidentifier(): + continue + name = token.rsplit("/", 1)[-1] + if name in {"env", "command", "builtin", "--"}: + continue + return tokens[index:] + return [] + +def _has_behavior_command(tokens: list[str]) -> bool: + words = _command_words(tokens) + return bool(words) and not words[0].startswith("-") and words[0].rsplit("/", 1)[-1] not in { + "true", ":", "echo", "printf", + } + def _verification_mode(command: str) -> bool: try: - tokens = shlex.split(command) + segments = [shlex.split(segment) for segment in _command_segments(command)] except ValueError: return False - return bool(tokens) and not any( + return any(_has_behavior_command(tokens) for tokens in segments) and not any( token in {"-h", "--help", "--collect-only", "--co", "--version"} - or token.startswith("--collect-only=") for token in tokens - ) and tokens[0] not in {"true", ":", "echo", "printf"} + or token.startswith("--collect-only=") for tokens in segments for token in tokens + ) async def _verify_commands(ctx: Any, commands: Sequence[Any], seconds: float | None) -> dict[str, Any]: deadline = math.inf if seconds is None else time.monotonic() + seconds @@ -103,7 +164,10 @@ async def _verify_commands(ctx: Any, commands: Sequence[Any], seconds: float | N checks.append(row) continue tool = _CommandProbe() - params: dict[str, Any] = {"command": command} + executed_command = tool.prepare_command(command) + if executed_command != command: + row["executed_command"] = executed_command + params: dict[str, Any] = {"command": executed_command} if math.isfinite(remaining): params["timeout"] = remaining row["output"] = await ctx.execute_verification(tool, params) @@ -116,16 +180,19 @@ async def _verify_commands(ctx: Any, commands: Sequence[Any], seconds: float | N truncated = any(getattr(result, key, False) for key in ( "stdout_truncated", "stderr_truncated", "stdout_dropped_bytes", "stderr_dropped_bytes", )) + completed = result.returncode == 0 and not truncated and not getattr(result, "timed_out", False) if records: - row["ok"] = all(record.get("verified") is True for record in records) - elif expected is None: - row["reason"] = "command requires parsed test evidence or an explicit expected_output probe" + row["ok"] = completed and all(record.get("verified") is True for record in records) + requires_test_evidence = tool.requires_test_evidence(command) + if not records and (expected is None or requires_test_evidence): + row["reason"] = ("pytest command requires per-target test evidence from a supported invocation" + if requires_test_evidence else + "command requires parsed test evidence or an explicit expected_output probe") if expected is not None: output = result.stdout + ("\n" + result.stderr if result.stderr else "") row["expected_output"] = expected row["matched"] = expected in output - row["ok"] = (row["ok"] if records else True) and result.returncode == 0 \ - and not truncated and not getattr(result, "timed_out", False) and row["matched"] + row["ok"] = (row["ok"] if records else not requires_test_evidence) and completed and row["matched"] checks.append(row) return { "ok": bool(checks) and all(row["ok"] for row in checks), diff --git a/tests/agents/test_session_snapshot_model_lifetime.py b/tests/agents/test_session_snapshot_model_lifetime.py new file mode 100644 index 00000000..9d0fea57 --- /dev/null +++ b/tests/agents/test_session_snapshot_model_lifetime.py @@ -0,0 +1,318 @@ +from __future__ import annotations + +import asyncio +import gc +import itertools +import json +import os +import shutil +import tempfile +import threading +import weakref +from pathlib import Path + +import httpx +import openai +import pytest + +from opencollab.adapters.env import LocalEnvironment +from opencollab.adapters.llm.client import LLMClient +from opencollab.application.event_bus import EventBus +from opencollab.bootstrap import DefaultSessionFactory, SnapshotSessionError, build_session, container, snapshot_session +from opencollab.bootstrap.context_builder import SpawnConfig +from opencollab.bootstrap.team_config import RoleConfig, TeamConfig +from opencollab.domain.agent import Agent +from tests.support.session_characterization_test_support import llm_response + + +class _ForkableEnvironment(LocalEnvironment): + def __init__(self, workspace): + super().__init__(str(workspace)) + self.forks = [] + + def fork_snapshot(self): + directory = tempfile.mkdtemp(prefix="snapshot-", dir=Path(self.workspace).parent) + shutil.copytree(self.workspace, directory, dirs_exist_ok=True) + branch = type(self)(directory) + self.forks.append(branch) + return branch + + +class _NativeTraffic: + def __init__(self, monkeypatch): + self.requests = [] + self.clients = [] + self.close_calls = 0 + self.started = asyncio.Event() + self.release = asyncio.Event() + traffic = self + native = openai.AsyncOpenAI + + class Transport(httpx.MockTransport): + async def aclose(self): + traffic.close_calls += 1 + await super().aclose() + + def client(**kwargs): + sdk = native(**kwargs, http_client=httpx.AsyncClient(transport=Transport(self.respond))) + self.clients.append(sdk) + return sdk + + monkeypatch.setattr(openai, "AsyncOpenAI", client) + + async def respond(self, request): + data = json.loads(request.content) + self.requests.append(data) + prompt = next(row["content"] for row in reversed(data["messages"]) if row["role"] == "user") + if prompt == "HOLD_BRANCH": + self.started.set() + await self.release.wait() + return httpx.Response(200, json={ + "id": f"chatcmpl-{len(self.requests)}", "object": "chat.completion", "created": 1, + "model": "gpt-4o", "choices": [{"index": 0, "finish_reason": "stop", + "message": {"role": "assistant", "content": "ANSWER " + prompt}}], + "usage": {"prompt_tokens": 11, "completion_tokens": 7, "total_tokens": 18}, + }) + + +@pytest.fixture(autouse=True) +def _quiet_runtime(monkeypatch): + monkeypatch.setenv("OPENCOLLAB_API_USAGE_LOG", os.devnull) + monkeypatch.setenv("OPENCOLLAB_BUDGET_NUDGE_MODE", "off") + monkeypatch.setenv("OPENCOLLAB_WRITE_NUDGE_MODE", "off") + + +def _native_source(tmp_path, *, borrowed=False): + workspace = tmp_path / "source" + workspace.mkdir() + environment = _ForkableEnvironment(workspace) + config = SpawnConfig( + model="gpt-4o", provider="openai", api_key="offline-test-key", # pragma: allowlist secret + base_url="http://127.0.0.1:9/v1", llm_timeout=3, llm_max_retries=0, + tracer=None, event_bus=EventBus(), permission_policy=None, thinking=False, + ) + factory = DefaultSessionFactory( + config, team_cfg=TeamConfig(roles={"worker": RoleConfig(prompt="Return the marker.", tools=[])}), + lead_workspace=str(workspace), lead_environment=environment, + ) + if not borrowed: + return factory.build_spawn_session(role="worker", env=environment, budget=100_000), None + model = LLMClient( + model="gpt-4o", provider="openai", api_key="offline-test-key", # pragma: allowlist secret + base_url="http://127.0.0.1:9/v1", max_retries=0, request_timeout=3, + ) + agent = Agent(name="worker", system_prompt="Return the marker.", model="gpt-4o", thinking=False) + return build_session(agent=agent, llm=model, env=environment, max_budget_tokens=100_000), model + + +async def _turn(session, marker): + await session.add_user_message(marker) + assert await session.run_loop() == "ANSWER " + marker + + +async def _cleanup(*sessions): + for session in sessions: + await session.aclose() + await session.env.cleanup() + + +@pytest.mark.parametrize("borrowed", [False, True]) +@pytest.mark.parametrize("first", ["source", "branch", "alive"]) +async def test_native_snapshot_model_survives_either_session_close(tmp_path, monkeypatch, borrowed, first): + traffic = _NativeTraffic(monkeypatch) + source, external = _native_source(tmp_path, borrowed=borrowed) + branch = None + try: + await _turn(source, "SOURCE_READY") + branch = snapshot_session(source) + assert branch._llm is source._llm + assert len(traffic.clients) == 1 + await branch.env.write_file("marker.txt", "BRANCH_MARKER") + await _turn(branch, "BRANCH_READY") + if first == "alive": + await _turn(source, "SOURCE_STILL_ALIVE") + await _turn(branch, "BRANCH_STILL_ALIVE") + else: + closed, remaining = (source, branch) if first == "source" else (branch, source) + await _cleanup(closed) + assert not traffic.clients[0].is_closed() + before = len(traffic.requests) + await _turn(remaining, "AFTER_CLOSE") + assert len(traffic.requests) == before + 1 + if first == "source": + assert await branch.env.read_file("marker.txt") == "BRANCH_MARKER" + finally: + await _cleanup(*[session for session in (source, branch) if session is not None]) + if external is not None: + assert not traffic.clients[0].is_closed() + assert traffic.close_calls == 0 + await external.close() + assert traffic.clients[0].is_closed() + assert traffic.close_calls == 1 + assert all(session.env._workspace_fd is None for session in (source, branch)) + assert all(not session.pending_cleanup_tasks for session in (source, branch)) + + +async def test_source_close_keeps_inflight_native_snapshot_request_alive(tmp_path, monkeypatch): + traffic = _NativeTraffic(monkeypatch) + source, _ = _native_source(tmp_path) + branch = snapshot_session(source) + running = asyncio.create_task(_turn(branch, "HOLD_BRANCH")) + try: + await asyncio.wait_for(traffic.started.wait(), 2) + await _cleanup(source) + assert not traffic.clients[0].is_closed() + traffic.release.set() + await asyncio.wait_for(running, 2) + await _turn(branch, "AFTER_INFLIGHT") + assert len(traffic.requests) == 2 + finally: + traffic.release.set() + await asyncio.gather(running, return_exceptions=True) + await _cleanup(branch, source) + assert traffic.clients[0].is_closed() + assert traffic.close_calls == 1 + + +class _Model: + def __init__(self, *, error=None): + self.close_calls = 0 + self.error = error + + async def complete(self, messages, **kwargs): + assert self.close_calls == 0 + return llm_response(content="ANSWER " + messages[-1]["content"]) + + async def close(self): + self.close_calls += 1 + await asyncio.sleep(0) + if self.error is not None: + raise self.error + + +def _owned_source(tmp_path, monkeypatch, model): + monkeypatch.setattr(container, "LLMClient", lambda **_kwargs: model) + workspace = tmp_path / "source" + workspace.mkdir() + return build_session( + agent=Agent(name="worker", system_prompt="Return the marker."), + env=_ForkableEnvironment(workspace), + ) + + +@pytest.mark.parametrize("order", list(itertools.permutations(range(4)))) +async def test_snapshot_siblings_and_descendants_close_model_after_last_owner(tmp_path, monkeypatch, order): + model = _Model() + source = _owned_source(tmp_path, monkeypatch, model) + branch = snapshot_session(source) + sessions = [source, branch, snapshot_session(source), snapshot_session(branch)] + try: + for index in order[:-1]: + await sessions[index].aclose() + await sessions[index].aclose() + assert model.close_calls == 0 + await _turn(sessions[order[-1]], "LAST_OWNER") + await sessions[order[-1]].aclose() + assert model.close_calls == 1 + finally: + await _cleanup(*sessions) + assert model.close_calls == 1 + + +async def test_concurrent_snapshot_closes_release_each_owner_once(tmp_path, monkeypatch): + model = _Model() + source = _owned_source(tmp_path, monkeypatch, model) + branch = snapshot_session(source) + descendant = snapshot_session(branch) + try: + await asyncio.gather(*(session.aclose() for session in (source, branch, source, branch))) + assert model.close_calls == 0 + await _turn(descendant, "LAST_OWNER") + await asyncio.gather(descendant.aclose(), descendant.aclose()) + assert model.close_calls == 1 + finally: + await _cleanup(descendant, branch, source) + + +@pytest.mark.parametrize("failure", ["fork", "context"]) +async def test_failed_snapshot_leaves_no_model_owner(tmp_path, monkeypatch, failure): + model = _Model() + source = _owned_source(tmp_path, monkeypatch, model) + if failure == "fork": + def fail_fork(): + raise OSError("fork failed") + monkeypatch.setattr(source.env, "fork_snapshot", fail_fork) + else: + class LockedShaper: + def __init__(self): + self.lock = threading.Lock() + source.runner.shaper = LockedShaper() + try: + with pytest.raises(SnapshotSessionError): + snapshot_session(source) + assert all(environment._workspace_fd is None for environment in source.env.forks) + await source.aclose() + assert model.close_calls == 1 + finally: + await _cleanup(source) + + +async def test_live_snapshot_does_not_retain_closed_source_session(tmp_path, monkeypatch): + model = _Model() + source = _owned_source(tmp_path, monkeypatch, model) + branch = snapshot_session(source) + source_ref = weakref.ref(source) + await _cleanup(source) + del source + gc.collect() + descendant = None + try: + assert source_ref() is None + assert model.close_calls == 0 + await _turn(branch, "SURVIVING_BRANCH") + descendant = snapshot_session(branch) + await _cleanup(branch) + assert model.close_calls == 0 + await _turn(descendant, "SURVIVING_DESCENDANT") + finally: + await _cleanup(branch) + if descendant is not None: + await _cleanup(descendant) + assert model.close_calls == 1 + + +@pytest.mark.parametrize("error", [OSError("close failed"), asyncio.CancelledError("close cancelled")]) +async def test_last_snapshot_close_retains_model_close_failure(tmp_path, monkeypatch, error): + model = _Model(error=error) + source = _owned_source(tmp_path, monkeypatch, model) + branch = snapshot_session(source) + try: + await source.aclose() + assert model.close_calls == 0 + for _ in range(2): + with pytest.raises(type(error)) as caught: + await branch.aclose() + assert caught.value is error + assert model.close_calls == 1 + await source.aclose() + finally: + await source.env.cleanup() + await branch.env.cleanup() + + +@pytest.mark.parametrize("has_branch", [False, True]) +async def test_snapshot_refuses_released_model_owner(tmp_path, monkeypatch, has_branch): + model = _Model() + source = _owned_source(tmp_path, monkeypatch, model) + branch = snapshot_session(source) if has_branch else None + try: + await source.aclose() + before = len(source.env.forks) + with pytest.raises(SnapshotSessionError, match="closed"): + snapshot_session(source) + assert len(source.env.forks) == before + if branch is not None: + await _turn(branch, "SURVIVING_BRANCH") + finally: + await _cleanup(*[session for session in (source, branch) if session is not None]) + assert model.close_calls == 1 diff --git a/tests/agents/test_session_snapshot_observation.py b/tests/agents/test_session_snapshot_observation.py new file mode 100644 index 00000000..92afac6f --- /dev/null +++ b/tests/agents/test_session_snapshot_observation.py @@ -0,0 +1,268 @@ +from __future__ import annotations + +import errno +import json +import os +import shutil +import sys +import tempfile +import threading +from pathlib import Path + +import httpx +import openai +import pytest + +from opencollab.adapters.env import LocalEnvironment +from opencollab.adapters.trace import Tracer +from opencollab.application.event_bus import EventBus +from opencollab.application.tool_execution import ToolRuntime +from opencollab.bootstrap import DefaultSessionFactory, SnapshotSessionError, session_factory, snapshot_session +from opencollab.bootstrap.context_builder import SpawnConfig +from opencollab.bootstrap.team_config import RoleConfig, TeamConfig + + +class _ForkableEnvironment(LocalEnvironment): + def __init__(self, workspace): + super().__init__(str(workspace)) + self.forks = [] + + def fork_snapshot(self): + directory = tempfile.mkdtemp(prefix="snapshot-", dir=Path(self.workspace).parent) + shutil.copytree(self.workspace, directory, dirs_exist_ok=True) + branch = type(self)(directory) + self.forks.append(branch) + return branch + + +@pytest.fixture +def native_requests(monkeypatch): + requests, clients = [], [] + sdk_type = openai.AsyncOpenAI + + def respond(request): + payload = json.loads(request.content) + requests.append(payload) + messages = payload["messages"] + if messages[-1]["role"] == "tool": + message = {"role": "assistant", "content": "RESULT " + messages[-1]["content"]} + reason = "stop" + elif "CHILD_READ" in str(messages): + message = {"role": "assistant", "content": None, "tool_calls": [{ + "id": f"read-{len(requests)}", "type": "function", "function": { + "name": "file_read", "arguments": json.dumps({"path": "marker.txt"}), + }, + }]} + reason = "tool_calls" + else: + message, reason = {"role": "assistant", "content": "PARENT_READY"}, "stop" + return httpx.Response(200, json={ + "id": f"chatcmpl-{len(requests)}", "object": "chat.completion", "created": 1, + "model": "gpt-4o", "choices": [{"index": 0, "message": message, "finish_reason": reason}], + "usage": {"prompt_tokens": 11, "completion_tokens": 7, "total_tokens": 18}, + }) + + def native_client(**kwargs): + client = sdk_type(**kwargs, http_client=httpx.AsyncClient(transport=httpx.MockTransport(respond))) + clients.append(client) + return client + + monkeypatch.setattr(openai, "AsyncOpenAI", native_client) + monkeypatch.setenv("PYTHONPATH", str(Path(session_factory.__file__).resolve().parents[2])) + monkeypatch.setenv("OPENCOLLAB_API_USAGE_LOG", os.devnull) + monkeypatch.setenv("OPENCOLLAB_BUDGET_NUDGE_MODE", "off") + monkeypatch.setenv("OPENCOLLAB_WRITE_NUDGE_MODE", "off") + monkeypatch.setenv("OPENCOLLAB_WORKFLOWS_DIR", "workflows") + return requests, clients + + +def _factory(tmp_path, source_events): + workspace = tmp_path / "source" + workspace.mkdir() + (workspace / "marker.txt").write_text("SOURCE_MARKER") + (workspace / "authored.py").write_text('''import os +from pathlib import Path +from opencollab.adapters.tools.fs import FileReadTool + +async def main(ctx, args): + Path("child-pid.txt").write_text(str(os.getpid())) + answer = await ctx.agent("CHILD_READ", tools=[FileReadTool()]) + Path("child-result.txt").write_text(answer) + if args.get("fail"): + raise RuntimeError("authored failure after native child") + return answer +''') + workflows = workspace / "workflows" + workflows.mkdir() + (workflows / "read_marker.py").write_text('''from opencollab import workflow +from opencollab.adapters.tools.fs import FileReadTool + +@workflow(name="read-marker", description="Read the local marker") +async def main(ctx, args): + return await ctx.agent("CHILD_READ", tools=[FileReadTool()]) +''') + tracer = Tracer("source", output_dir=str(tmp_path / "trace")) + bus = EventBus(source_events.append) + config = SpawnConfig( + model="gpt-4o", provider="openai", api_key="offline-test", # pragma: allowlist secret + base_url="http://127.0.0.1:9/v1", llm_timeout=3, llm_max_retries=0, + tracer=tracer, event_bus=bus, permission_policy=None, thinking=False, + temperature=0.35, top_p=0.8, max_output_tokens=128, workflow_cleanup_timeout=0.75, + thinking_params={"budget_tokens": 512, "metadata": {"labels": ["source"]}}, + model_routes={"reader": {"model": "gpt-4o", "provider": "openai"}}, + ) + environment = _ForkableEnvironment(workspace) + save_dir = tmp_path / "saved" + save_dir.mkdir() + factory = DefaultSessionFactory( + config, team_cfg=TeamConfig(roles={"worker": RoleConfig( + prompt="Read the local marker when requested.", + tools=["bash", "file_read", "manage_workflow", "run_workflow"], + )}), lead_workspace=str(workspace), lead_environment=environment, + save_dir=str(save_dir), allow_unisolated_child_shell=True, + ) + source = factory.build_spawn_session(role="worker", env=environment, budget=100_000, aid=0) + return source, config + + +def _trace_lines(tracer): + tracer.flush() + return Path(tracer.path).read_bytes().splitlines() + + +def _saved_files(session): + return {suffix: Path(str(session.auto_save_path) + suffix).read_bytes() for suffix in ("", ".journal")} + + +async def _cleanup(source, branch, tracer, clients): + for session in (branch, source): + if session is not None: + await session.aclose() + descriptor = session.env._workspace_fd + await session.env.cleanup() + with pytest.raises(OSError) as caught: + os.fstat(descriptor) + assert caught.value.errno == errno.EBADF + for client in clients: + await client.close() + assert client.is_closed() + tracer.close() + assert tracer._file.closed + assert not tracer._writer.is_alive() + + +@pytest.mark.parametrize("workflow", ["authored", "authored-error", "registered"]) +@pytest.mark.parametrize("observed", [False, True]) +async def test_factory_traced_snapshot_runs_workflows_with_independent_observation( + tmp_path, native_requests, workflow, observed, +): + requests, clients = native_requests + source_events, branch_events = [], [] + source, config = _factory(tmp_path, source_events) + tracer, bus = config.tracer, config.event_bus + branch = None + try: + await source.add_user_message("Prepare the source") + assert await source.run_loop() == "PARENT_READY" + await source.add_user_message("Resume this turn after snapshotting") + await source.enqueue_auto_save() + source_trace = _trace_lines(tracer) + source_saved = _saved_files(source) + source_event_count = len(source_events) + source_tokens = source.used_tokens + branch = snapshot_session(source, **({"event_sink": branch_events.append} if observed else {})) + assert branch.tracer is None + assert branch.auto_save_path is None + authored = branch.agent.find_tool("manage_workflow") + registered = branch.agent.find_tool("run_workflow") + assert authored._config is registered._config + assert authored is branch.runner._initial_agent_tools[-2] + assert registered is branch.runner._initial_agent_tools[-1] + copied = authored._config + assert copied is not config + assert copied.tracer is None + assert copied.event_bus.subscribers == () + assert copied.event_bus is not bus + assert copied.model_settings() == config.model_settings() + assert copied.model_settings("reader") == config.model_settings("reader") + for name in vars(config).keys() - {"tracer", "event_bus"}: + assert getattr(copied, name) == getattr(config, name) + copied.thinking_params["metadata"]["labels"].append("branch") + copied.model_routes["reader"]["model"] = "gpt-4o-mini" + assert config.thinking_params["metadata"]["labels"] == ["source"] + assert config.model_routes["reader"]["model"] == "gpt-4o" + assert config.tracer is tracer + assert config.event_bus is bus + + await branch.env.write_file("marker.txt", "BRANCH_MARKER") + assert await branch.run_loop() == "PARENT_READY" + await branch.add_user_message("Run the workflow") + runtime = ToolRuntime(branch.env, None, None) + if workflow.startswith("authored"): + launched = json.loads(await authored.execute_with_runtime({ + "action": "launch", "source": "authored.py", "python": sys.executable, "budget": 20_000, + "args": {"fail": workflow == "authored-error"}, + }, runtime)) + finished = json.loads(await authored.execute_with_runtime({ + "action": "wait", "id": launched["id"], "timeout": 15, + }, runtime)) + if workflow == "authored-error": + assert finished["status"] == "failed", finished + assert "authored failure after native child" in finished["result"]["reason"] + else: + assert finished["status"] == "completed", finished + assert "BRANCH_MARKER" in finished["result"]["output"] + assert "BRANCH_MARKER" in await branch.env.read_file("child-result.txt") + pid = int(await branch.env.read_file("child-pid.txt")) + with pytest.raises(ProcessLookupError): + os.kill(pid, 0) + child = authored._children[launched["id"]] + assert child.execution.done() + assert authored._reserved_tokens() == 0 + else: + finished = json.loads(await registered.execute_with_runtime({ + "name": "read-marker", "args": {}, "budget": 20_000, + }, runtime)) + assert "BRANCH_MARKER" in finished["output"] + assert finished["tokens"] == 36 + assert len(requests) == 4 + assert all(request["model"] == "gpt-4o" for request in requests) + assert all(request["temperature"] == 0.35 and request["top_p"] == 0.8 for request in requests) + assert source.used_tokens == source_tokens == 18 + assert branch.used_tokens == 72 + assert await source.env.read_file("marker.txt") == "SOURCE_MARKER" + assert len(source_events) == source_event_count + assert bool(branch_events) is observed + assert _trace_lines(tracer) == source_trace + assert _saved_files(source) == source_saved + await branch.aclose() + assert not tracer._closed + assert tracer._writer.is_alive() + assert await source.run_loop() == "PARENT_READY" + await source.enqueue_auto_save() + assert source.used_tokens == 36 + assert len(_trace_lines(tracer)) > len(source_trace) + assert _saved_files(source) != source_saved + assert len(source_events) > source_event_count + finally: + await _cleanup(source, branch, tracer, clients) + + +async def test_uncopyable_workflow_settings_fail_before_allocating_a_snapshot(tmp_path, native_requests): + requests, clients = native_requests + source, config = _factory(tmp_path, []) + tracer = config.tracer + try: + config.model_routes["uncloneable"] = threading.Lock() + with pytest.raises(SnapshotSessionError, match="workflow configuration") as caught: + snapshot_session(source) + assert isinstance(caught.value.__cause__, TypeError) + assert source.env.forks == [] + assert config.tracer is tracer + assert tracer._writer.is_alive() + await source.add_user_message("Continue after the rejected snapshot") + assert await source.run_loop() == "PARENT_READY" + assert len(requests) == 1 + assert _trace_lines(tracer) + finally: + await _cleanup(source, None, tracer, clients) diff --git a/tests/agents/test_session_snapshot_runtime.py b/tests/agents/test_session_snapshot_runtime.py new file mode 100644 index 00000000..4fde7bba --- /dev/null +++ b/tests/agents/test_session_snapshot_runtime.py @@ -0,0 +1,574 @@ +from __future__ import annotations + +import asyncio +import copy +import json +import os +import shutil +import sys +import tempfile +from functools import partial +from pathlib import Path + +import pytest + +from opencollab.adapters.env import LocalEnvironment +from opencollab.adapters.safety import SandboxInterceptor +from opencollab.adapters.session_environments import SessionEnvironmentPool +from opencollab.adapters.tools.bash import BashTool +from opencollab.adapters.tools.fs import FileReadTool, FileWriteTool +from opencollab.application.event_bus import EventBus +from opencollab.application.run_control import BudgetDecision, RunControl +from opencollab.application.submit_findings import SubmitFindingsTool +from opencollab.application.tool_execution import ToolRuntime +from opencollab.application.workflow_agents import WorkflowAgentsMixin +from opencollab.application.workflow_registry import Registry, WorkflowSpec +from opencollab.bootstrap import DefaultSessionFactory, build_session, snapshot_session +from opencollab.bootstrap.agent_profiles import resolve_agent_profile +from opencollab.bootstrap.authored_workflow import AuthoredWorkflowTool +from opencollab.bootstrap.context_builder import SpawnConfig +from opencollab.bootstrap.context_policy import ContextPolicy +from opencollab.bootstrap.team_config import RoleConfig, TeamConfig +from opencollab.bootstrap.team_workflow import TeamWorkflowTool +from opencollab.domain.agent import Agent +from tests.support.session_characterization_test_support import llm_response + + +class _ForkableEnvironment(LocalEnvironment): + def fork_snapshot(self): + directory = tempfile.mkdtemp(prefix="snapshot-", dir=Path(self.workspace).parent) + shutil.copytree(self.workspace, directory, dirs_exist_ok=True) + return type(self)(directory) + + +@pytest.fixture(autouse=True) +def _quiet_nudges(monkeypatch): + monkeypatch.setenv("OPENCOLLAB_BUDGET_NUDGE_MODE", "off") + monkeypatch.setenv("OPENCOLLAB_WRITE_NUDGE_MODE", "off") + + +def _call(name, arguments, ident): + return { + "id": ident, "type": "function", + "function": {"name": name, "arguments": json.dumps(arguments)}, + } + + +async def _close(*sessions): + for session in sessions: + if session is not None: + await session.aclose() + await session.env.cleanup() + + +class _FileModel: + def __init__(self): + self.calls = [] + + async def complete(self, messages, **kwargs): + self.calls.append(copy.deepcopy(messages)) + user = max(i for i, row in enumerate(messages) if row["role"] == "user") + results = [row["content"] for row in messages[user + 1:] if row["role"] == "tool"] + if results: + return llm_response(content=results[-1], input_tokens=9, output_tokens=4) + prompt = messages[user]["content"] + if "Create" in prompt: + name, args = "file_write", {"path": "created.txt", "mode": "create", "content": "CREATED"} + elif "Edit" in prompt: + name, args = "file_write", { + "path": "marker.txt", "mode": "str_replace", "old_str": "EXPECTED_TAIL", "new_str": "EDITED_TAIL", + } + else: + name, args = "file_read", {"path": "marker.txt"} + return llm_response(tool_calls=[_call(name, args, f"file-{len(self.calls)}")], + finish_reason="tool_calls", input_tokens=7, output_tokens=3) + + +@pytest.mark.parametrize("hard_budget", [50_000, None]) +async def test_snapshot_preserves_dynamic_budget_and_its_current_allowance(tmp_path, hard_budget): + workspace = tmp_path / "source" + workspace.mkdir() + (workspace / "marker.txt").write_text("BUDGET_MARKER") + decisions, events = [], [] + + def decide(snapshot): + decisions.append(snapshot) + return BudgetDecision(soft_budget_tokens=10_000) + + source = build_session( + agent=Agent(name="reader", system_prompt="Read the marker.", tools=[FileReadTool()]), + env=_ForkableEnvironment(str(workspace)), llm=_FileModel(), max_budget_tokens=hard_budget, + run_control=RunControl( + initial_soft_budget_tokens=500, decide_budget=decide, on_event=events.append, + history_trigger_tokens=20_000, tool_cancellation_cleanup_timeout=0.75, + ), + ) + source.max_budget_tokens = 1000 + branch = None + try: + branch = snapshot_session(source) + task = "Read marker.txt.\n" + "Additional local task context. " * 300 + for session in (source, branch): + before = len(source._llm.calls) + await session.add_user_message(task) + answer = await session.run_loop() + assert "BUDGET_MARKER" in answer, (session.state.terminal_reason, session.messages[-3:]) + assert len(source._llm.calls) - before == 2 + assert session.runner.hard_budget_tokens == hard_budget + assert session.max_budget_tokens == 10_000 + assert session.used_tokens == 23 + assert branch.runner._run_control.initial_soft_budget_tokens == 1000 + assert branch.runner._run_control.decide_budget is decide + assert branch.runner._run_control.on_event is None + assert branch.runner._run_control.history_trigger_tokens == 20_000 + assert branch.tool_execution._cancellation_cleanup_timeout == 0.75 + source_ids = {event.session_id for event in events} + assert source_ids == {source.runner._response_session_id} + assert {decision.session_id for decision in decisions} == { + source.runner._response_session_id, branch.runner._response_session_id, + } + finally: + await _close(branch, source) + + +async def test_snapshot_preserves_an_unbounded_allowance(tmp_path): + (tmp_path / "marker.txt").write_text("UNBOUNDED_MARKER") + source = build_session( + agent=Agent(name="reader", system_prompt="Read.", tools=[FileReadTool()]), + env=_ForkableEnvironment(str(tmp_path)), llm=_FileModel(), max_budget_tokens=None, + run_control=RunControl(decide_budget=lambda snapshot: BudgetDecision(None)), + ) + branch = None + try: + branch = snapshot_session(source) + assert branch.max_budget_tokens is None + assert branch.runner.hard_budget_tokens is None + assert branch.runner._run_control.decide_budget is source.runner._run_control.decide_budget + await branch.add_user_message("Read marker.txt.") + assert "UNBOUNDED_MARKER" in await branch.run_loop() + assert branch.max_budget_tokens is None + finally: + await _close(branch, source) + + +@pytest.mark.parametrize("policy_alias", [False, True]) +async def test_single2_snapshot_reads_creates_and_edits_its_own_workspace(tmp_path, policy_alias): + workspace = tmp_path / "source" + workspace.mkdir() + (workspace / "marker.txt").write_text("HEAD_" + "x" * 1600 + "_EXPECTED_TAIL\n") + allowed = tmp_path / "allowed" + allowed.mkdir() + policy_root = workspace + if policy_alias: + policy_root = tmp_path / "source-alias" + policy_root.symlink_to(workspace, target_is_directory=True) + source = build_session( + agent=Agent(name="reader", system_prompt="Read and edit the requested file.", + tools=[FileReadTool(), FileWriteTool()]), + env=_ForkableEnvironment(str(workspace)), llm=_FileModel(), + safety_policy=SandboxInterceptor(str(policy_root), allowed_dirs=[str(allowed)]), + agent_profile=resolve_agent_profile("single2"), + context_policy=ContextPolicy("no_history_compaction", 240), + ) + branch = None + try: + await source.add_user_message("Read the marker.") + answer = await source.run_loop() + assert "EXPECTED_TAIL" in answer, (source.state.terminal_reason, source.messages[-3:]) + source_view = source._llm.calls[-1][-1]["content"] + branch = snapshot_session(source) + await branch.add_user_message("Read the marker in the branch.") + assert "EXPECTED_TAIL" in await branch.run_loop() + assert source._llm.calls[-1][-1]["content"] == source_view + assert len(source_view) == 240 + for task in ("Create a file in the branch.", "Edit the marker in the branch."): + await branch.add_user_message(task) + assert "Error" not in await branch.run_loop() + assert await branch.env.read_file("created.txt") == "CREATED" + assert "EDITED_TAIL" in await branch.env.read_file("marker.txt") + assert not (workspace / "created.txt").exists() + assert "EXPECTED_TAIL" in (workspace / "marker.txt").read_text() + policy = branch.tool_execution.safety_policy + assert policy.check_path("marker.txt") == str(Path(branch.env.workspace) / "marker.txt") + assert policy.check_path(str(allowed / "scratch")) == str(allowed / "scratch") + with pytest.raises(PermissionError, match="escapes workspace"): + policy.check_path(str(workspace / "marker.txt")) + with pytest.raises(PermissionError, match="Blocked container-wide"): + policy.check_cmd("find /") + with pytest.raises(PermissionError, match="Blocked destructive"): + policy.check_cmd("rm -rf /") + finally: + await _close(branch, source) + + +class _ScoutModel: + def __init__(self): + self.calls = [] + + async def complete(self, messages, tools=None, **kwargs): + names = [tool["function"]["name"] for tool in tools or []] + self.calls.append((names, copy.deepcopy(kwargs.get("tool_choice")))) + if names == ["submit_findings"]: + return llm_response(tool_calls=[_call("submit_findings", { + "findings": [], "summary": "No task evidence has been read.", "insufficient_evidence": True, + }, f"submit-{len(self.calls)}")], finish_reason="tool_calls") + return llm_response(content="READY") + + +class _ReadOrCommitModel(_FileModel): + def __init__(self): + super().__init__() + self.requests = [] + + async def complete(self, messages, tools=None, **kwargs): + names = [tool["function"]["name"] for tool in tools or []] + self.requests.append((names, copy.deepcopy(kwargs.get("tool_choice")))) + if names == ["submit_findings"]: + return llm_response(tool_calls=[_call("submit_findings", { + "findings": [], "summary": "No matching evidence was collected for this turn.", + "insufficient_evidence": True, + }, f"submit-{len(self.requests)}")], finish_reason="tool_calls") + return await super().complete(messages, tools=tools, **kwargs) + + +@pytest.mark.parametrize("expansion", ["direct", "policy"]) +@pytest.mark.parametrize("expanded_cap,forced", [(25_000, True), (50_000, False)]) +async def test_snapshot_preserves_reserve_after_shrink_and_expansion( + tmp_path, expansion, expanded_cap, forced, +): + workspace = tmp_path / "source" + workspace.mkdir() + (workspace / "marker.txt").write_text("RESERVE_MARKER") + target = None + + def decide(observation): + return BudgetDecision(observation.soft_budget_tokens if target is None else target) + + model = _ReadOrCommitModel() + source = build_session( + agent=Agent(name="scout", system_prompt="Read evidence and submit findings.", + tools=[FileReadTool(), SubmitFindingsTool()]), + env=_ForkableEnvironment(str(workspace)), llm=model, max_budget_tokens=100_000, + run_control=RunControl(decide_budget=decide) if expansion == "policy" else None, + ) + branch = None + try: + source.runner.configure_enforcement(enforcement_strength="needs-enforcement", commit_reserve=25_000) + await source.add_user_message("Read marker.txt.") + assert "RESERVE_MARKER" in await source.run_loop() + assert source.used_tokens == 23 + source.max_budget_tokens = 10_000 + branch = snapshot_session(source) + if expansion == "direct": + source.max_budget_tokens = branch.max_budget_tokens = expanded_cap + else: + target = expanded_cap + for session in (source, branch): + before = len(model.requests) + message_start = len(session.messages) + await session.add_user_message("Read marker.txt and report the available evidence.") + answer = await session.run_loop() + choice = {"type": "function", "function": {"name": "submit_findings"}} + expected = (["submit_findings"], choice) if forced else (["file_read", "submit_findings"], None) + assert model.requests[before] == expected + assert (session.agent.find_tool("submit_findings").captured is not None) is forced + results = [row["content"] for row in session.messages[message_start:] if row["role"] == "tool"] + assert any("RESERVE_MARKER" in value for value in results) is (not forced) + if not forced: + assert "RESERVE_MARKER" in answer + assert session.max_budget_tokens == expanded_cap + finally: + await _close(branch, source) + + +@pytest.mark.parametrize("spent,forced", [(7499, False), (8001, True)]) +async def test_snapshot_preserves_scout_commit_threshold_and_capture(tmp_path, spent, forced): + model = _ScoutModel() + source = build_session( + agent=Agent(name="scout", system_prompt="Read evidence and submit findings.", + tools=[FileReadTool(), SubmitFindingsTool()]), + env=_ForkableEnvironment(str(tmp_path)), llm=model, max_budget_tokens=10_000, + ) + WorkflowAgentsMixin._configure_session_enforcement(source, "needs-enforcement", 2500) + source.used_tokens = spent + branch = None + try: + branch = snapshot_session(source) + for session in (source, branch): + before = len(model.calls) + await session.add_user_message("Read the marker and submit the available evidence.") + await session.run_loop() + expected = (["submit_findings"], {"type": "function", "function": {"name": "submit_findings"}}) + assert model.calls[before] == (expected if forced else (["file_read", "submit_findings"], None)) + assert (session.agent.find_tool("submit_findings").captured is not None) is forced + if not forced: + assert session.used_tokens == 7501 + before = len(model.calls) + await session.add_user_message("Submit the available evidence.") + await session.run_loop() + assert model.calls[before] == expected + assert session.agent.find_tool("submit_findings").captured is not None + if session is source: + assert branch.agent.find_tool("submit_findings").captured is None + assert branch.runner._enforcement_strength == "needs-enforcement" + assert branch.runner._commit_reserve == 2500 + assert branch.agent.find_tool("submit_findings") is not source.agent.find_tool("submit_findings") + finally: + await _close(branch, source) + + +async def _wait_file(path): + async def wait(): + while not path.exists(): + await asyncio.sleep(0.01) + await asyncio.wait_for(wait(), 10) + + +@pytest.mark.parametrize("close_first", ["source", "branch"]) +@pytest.mark.parametrize("narrowed", [False, True]) +async def test_snapshot_authored_workflow_processes_belong_to_each_session( + tmp_path, monkeypatch, close_first, narrowed, +): + monkeypatch.setenv("OPENCOLLAB_API_USAGE_LOG", os.devnull) + workspace = tmp_path / "source" + workspace.mkdir() + (workspace / "authored.py").write_text('''import asyncio +import os +from pathlib import Path + +async def main(ctx, args): + Path("entered").write_text(str(os.getpid())) + try: + await asyncio.sleep(300) + finally: + Path("cleaned").write_text("complete") +''') + config = SpawnConfig(model="gpt-4o", provider="openai", api_key=None, base_url=None, + llm_timeout=60, tracer=None, event_bus=EventBus(), permission_policy=None) + tool = AuthoredWorkflowTool(lambda: source, config, None, allow_unisolated_shell=True) + source = build_session( + agent=Agent(name="parent", system_prompt="Run a workflow.", tools=[BashTool(), tool]), + env=_ForkableEnvironment(str(workspace)), llm=_FileModel(), max_budget_tokens=1000, + ) + branch = None + try: + with pytest.raises(ValueError, match="unknown child"): + await tool.execute_with_runtime({"action": "status", "id": "unknown"}, ToolRuntime(source.env, None, None)) + if narrowed: + source.agent.tools = [] + branch = snapshot_session(source) + if narrowed: + assert branch.agent.tools == [] + for session in (source, branch): + await session.add_user_message("Run the owned workflow.") + branch_manager = branch.agent.find_tool("manage_workflow") + assert branch_manager is branch.runner._initial_agent_tools[-1] + children, pids = {}, {} + for label, session in (("source", source), ("branch", branch)): + manager = session.agent.find_tool("manage_workflow") + launched = json.loads(await manager.execute_with_runtime({ + "action": "launch", "source": "authored.py", "python": sys.executable, "budget": 400, + }, ToolRuntime(session.env, None, None))) + await _wait_file(Path(session.env.workspace) / "entered") + children[label] = manager._children[launched["id"]] + pids[label] = int((Path(session.env.workspace) / "entered").read_text()) + first, other = (source, branch) if close_first == "source" else (branch, source) + other_label = "branch" if close_first == "source" else "source" + await first.aclose() + assert children[close_first].execution.done() + assert (Path(first.env.workspace) / "cleaned").read_text() == "complete" + with pytest.raises(ProcessLookupError): + os.kill(pids[close_first], 0) + assert not children[other_label].execution.done() + os.kill(pids[other_label], 0) + assert other.runner.remaining_budget_tokens == 600 + await other.aclose() + assert children[other_label].execution.done() + assert (Path(other.env.workspace) / "cleaned").read_text() == "complete" + with pytest.raises(ProcessLookupError): + os.kill(pids[other_label], 0) + finally: + await _close(branch, source) + + +async def test_snapshot_registered_workflow_charges_its_own_session(tmp_path): + workspace = tmp_path / "source" + workspace.mkdir() + (workspace / "marker.txt").write_text("SOURCE_MARKER") + + async def read_marker(ctx, args): + return await ctx.agent("Read marker.txt.", tools=[FileReadTool()]) + + registry = Registry() + registry.register(WorkflowSpec("read-marker", "Read the marker.", read_marker)) + config = SpawnConfig(model="gpt-4o", provider="openai", api_key=None, base_url=None, + llm_timeout=60, tracer=None, event_bus=EventBus(), permission_policy=None) + tool = TeamWorkflowTool(lambda: source, registry, config, None) + source = build_session( + agent=Agent(name="parent", system_prompt="Run a workflow.", tools=[FileReadTool(), tool]), + env=_ForkableEnvironment(str(workspace)), llm=_FileModel(), max_budget_tokens=100_000, + ) + branch = None + try: + branch = snapshot_session(source) + await branch.env.write_file("marker.txt", "BRANCH_MARKER") + manager = branch.agent.find_tool("run_workflow") + assert manager is branch.runner._initial_agent_tools[-1] + response = json.loads(await manager.execute_with_runtime( + {"name": "read-marker", "args": {}}, ToolRuntime(branch.env, None, None), + )) + assert "BRANCH_MARKER" in response["output"] + assert response["tokens"] == branch.used_tokens == 23 + assert source.used_tokens == 0 + assert await source.env.read_file("marker.txt") == "SOURCE_MARKER" + finally: + await _close(branch, source) + + +class _SessionOwner: + def get_session(self): + return self.session + + +@pytest.mark.parametrize("tool_type", [AuthoredWorkflowTool, TeamWorkflowTool]) +async def test_snapshot_workflow_bound_method_keeps_its_owner_out_of_the_copy(tmp_path, tool_type): + owner = _SessionOwner() + config = SpawnConfig(model="gpt-4o", provider="openai", api_key=None, base_url=None, + llm_timeout=60, tracer=None, event_bus=EventBus(), permission_policy=None) + tool = ( + TeamWorkflowTool(owner.get_session, Registry(), config, None) + if tool_type is TeamWorkflowTool else AuthoredWorkflowTool(owner.get_session, config, None) + ) + source = build_session( + agent=Agent(name="parent", system_prompt="Run a workflow.", tools=[tool]), + env=_ForkableEnvironment(str(tmp_path)), llm=_FileModel(), + ) + owner.session = source + branch = None + try: + branch = snapshot_session(source) + manager = branch.agent.find_tool(tool.name) + assert manager is branch.runner._initial_agent_tools[0] + assert manager._get_session() is branch + assert tool._get_session() is source + finally: + await _close(branch, source) + + +@pytest.mark.parametrize("close_first", ["source", "branch"]) +@pytest.mark.parametrize("bound_factory", [False, True]) +async def test_snapshot_factory_preserves_environment_providers_and_current_catalog( + tmp_path, monkeypatch, close_first, bound_factory, +): + monkeypatch.setenv("PYTHONPATH", str(Path(__file__).resolve().parents[2])) + monkeypatch.setenv("OPENCOLLAB_API_USAGE_LOG", os.devnull) + workspace = tmp_path / "source" + workspace.mkdir() + (workspace / "marker.txt").write_text("SOURCE_MARKER\n") + (workspace / "authored.py").write_text( + 'from pathlib import Path\n' + 'async def main(ctx, args):\n' + ' marker = Path("marker.txt").read_text()\n' + ' Path("workflow-marker.txt").write_text(marker)\n' + ' return marker\n' + ) + environment = _ForkableEnvironment(str(workspace)) + pool = SessionEnvironmentPool(str(workspace), isolation="worktree", base_environment=environment) + pool.set_source(environment, isolation="shared") + provider = pool.child_pool if bound_factory else lambda env, *, role: pool.child_pool(env, role=role) + config = SpawnConfig(model="gpt-4o", provider="openai", api_key="offline-test", # pragma: allowlist secret + base_url="http://127.0.0.1:9/v1", llm_timeout=1, tracer=None, + event_bus=EventBus(), permission_policy=None) + factory = DefaultSessionFactory( + config, team_cfg=TeamConfig(roles={ + "worker": RoleConfig(prompt="Read the marker.", tools=["bash", "manage_workflow"]), + }), environment_pool_factory=provider, environment_catalog=pool.describe, + lead_workspace=str(workspace), lead_environment=environment, allow_unisolated_child_shell=True, + ) + source = factory.build_spawn_session(role="worker", env=environment, budget=10_000) + branch = None + try: + source_manager = source.agent.find_tool("manage_workflow") + assert source_manager._catalog()["default_isolation"] == "shared" + branch = snapshot_session(source) + await branch.env.write_file("marker.txt", "BRANCH_MARKER\n") + manager = branch.agent.find_tool("manage_workflow") + assert manager is branch.runner._initial_agent_tools[-1] + assert manager._get_session() is branch + assert manager._environment_pool_factory is provider + assert manager._catalog() == pool.describe(branch.env, role="worker") + schema = manager.to_openai_schema()["function"]["parameters"]["properties"]["isolation"] + assert schema["default"] == "worktree" + source_schema = source_manager.to_openai_schema()["function"]["parameters"]["properties"]["isolation"] + assert source_schema["default"] == "shared" + description = json.loads(await manager.execute_with_runtime( + {"action": "describe"}, ToolRuntime(branch.env, None, None), + )) + assert description["default_isolation"] == "worktree" + + child_pool = manager._environment_pool_factory(branch.env, role="worker") + try: + assert child_pool.default_isolation == "worktree" + child = await child_pool.acquire("child") + assert child is not branch.env + assert child.workspace != branch.env.workspace + assert await child.read_file("marker.txt") == "BRANCH_MARKER\n" + await child.write_file("marker.txt", "CHILD_MARKER\n") + outcome = await child.exec_cmd("cat marker.txt") + assert outcome.returncode == 0 and outcome.stdout == "CHILD_MARKER\n" + patch = await child.get_diff() + assert "-BRANCH_MARKER" in patch and "+CHILD_MARKER" in patch + assert await branch.env.read_file("marker.txt") == "BRANCH_MARKER\n" + assert await source.env.read_file("marker.txt") == "SOURCE_MARKER\n" + await child_pool.release_env(child) + finally: + await child_pool.release() + + first, other = (source, branch) if close_first == "source" else (branch, source) + await first.aclose() + await first.env.cleanup() + manager = other.agent.find_tool("manage_workflow") + runtime = ToolRuntime(other.env, None, None) + launched = json.loads(await manager.execute_with_runtime({ + "action": "launch", "source": "authored.py", "python": sys.executable, + }, runtime)) + finished = json.loads(await manager.execute_with_runtime({ + "action": "wait", "id": launched["id"], "timeout": 20, + }, runtime)) + expected = "BRANCH_MARKER\n" if other is branch else "SOURCE_MARKER\n" + assert finished["status"] == "completed", finished + assert finished["result"]["output"] == expected + assert await other.env.read_file("workflow-marker.txt") == expected + assert not (Path(first.env.workspace) / "workflow-marker.txt").exists() + child = manager._children[launched["id"]] + assert child.bridge._pool._source is other.env + assert child.bridge._pool.default_isolation == manager._catalog()["default_isolation"] + assert manager._reserved_tokens() == 0 + finally: + await _close(branch, source) + await pool.release() + + +async def test_snapshot_preserves_legacy_zero_argument_environment_catalog(tmp_path): + environment = _ForkableEnvironment(str(tmp_path)) + pool = SessionEnvironmentPool(str(tmp_path), isolation="worktree", base_environment=environment) + pool.set_source(environment, isolation="shared") + catalog = partial(pool.describe, environment, role="worker") + config = SpawnConfig(model="gpt-4o", provider="openai", api_key=None, base_url=None, + llm_timeout=60, tracer=None, event_bus=EventBus(), permission_policy=None) + tool = AuthoredWorkflowTool(lambda: source, config, None, environment_catalog=catalog) + source = build_session( + agent=Agent(name="worker", system_prompt="Run a workflow.", tools=[tool]), + env=environment, llm=_FileModel(), + ) + branch = None + try: + assert tool._catalog() == catalog() + branch = snapshot_session(source) + manager = branch.agent.find_tool("manage_workflow") + assert manager._environment_catalog is catalog + assert manager._catalog() == catalog() + assert manager.to_openai_schema()["function"]["parameters"]["properties"]["isolation"]["default"] == "shared" + finally: + await _close(branch, source) + await pool.release() diff --git a/tests/interfaces/test_authored_workflow_execution.py b/tests/interfaces/test_authored_workflow_execution.py new file mode 100644 index 00000000..d4b1faf9 --- /dev/null +++ b/tests/interfaces/test_authored_workflow_execution.py @@ -0,0 +1,133 @@ +from __future__ import annotations + +import json +import os +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from opencollab.adapters._env_process import ProcessCleanupError +from opencollab.adapters._team_candidate_environment import _TeamCandidateEnvironment +from opencollab.adapters.env import LocalEnvironment, WorktreeEnvironment +from tests.interfaces.test_member_authored_workflow import _launch, _manager +from tests.support.paths import REPO_ROOT + + +@pytest.fixture(autouse=True) +def _child_import_path(monkeypatch): + monkeypatch.setenv("PYTHONPATH", str(REPO_ROOT)) + monkeypatch.setenv("OPENCOLLAB_API_USAGE_LOG", os.devnull) + + +@pytest.mark.parametrize("kind", ["local", "copy-worktree", "git-worktree", "team-candidate"]) +@pytest.mark.parametrize("size", [1024, 4 * 1024 * 1024 + 1024]) +async def test_authored_result_and_native_artifacts_cross_file_read_limit(tmp_path, kind, size): + workspace = tmp_path / "workspace" + workspace.mkdir() + (workspace / "authored.py").write_text( + "async def main(ctx, args):\n return {'payload': args['payload']}\n" + ) + (workspace / ".gitignore").write_text(".opencollab/\n") + if kind == "git-worktree": + for args in (("init", "-q"), ("add", "."), ( + "-c", "user.name=Test", "-c", "user.email=test@example.invalid", + "commit", "-qm", "test: prepare workflow fixture", + )): + subprocess.run(["git", "-C", str(workspace), *args], check=True, capture_output=True) + environment = (LocalEnvironment if kind in {"local", "team-candidate"} else WorktreeEnvironment)(str(workspace)) + if kind == "team-candidate": + environment = _TeamCandidateEnvironment( + SimpleNamespace(environment=environment, cleanup=environment.cleanup), str(workspace), + ) + await environment.setup() + parent, tool, runtime = _manager(environment) + saved = tmp_path / "saved" + saved.mkdir() + tool._save_dir = str(saved) + payload = "x" * size + try: + child = await _launch(tool, runtime, args={"payload": payload}) + result = json.loads(await tool.execute_with_runtime({"action": "wait", "id": child["id"]}, runtime)) + native_path = Path(child["artifacts"]) / "result.json" + assert json.loads(native_path.read_text())["status"] == "completed" + assert result["status"] == "completed", result + assert result["result"]["output"] == {"payload": payload} + assert tool._reserved_tokens() == 0 + persisted = Path(result["artifacts"]) + assert json.loads((persisted / "result.json").read_text())["result"]["output"] == {"payload": payload} + assert json.loads((persisted / "workflow.json").read_text())["args"] == {"payload": payload} + if size > 4 * 1024 * 1024: + assert native_path.stat().st_size > 4 * 1024 * 1024 + assert (persisted / "workflow.json").stat().st_size > 4 * 1024 * 1024 + with pytest.raises(ValueError, match="read target exceeds"): + await environment.read_file(str(native_path)) + finally: + try: + await parent.aclose() + finally: + if isinstance(environment, WorktreeEnvironment): + await environment.get_diff() + await environment.cleanup() + + +@pytest.mark.parametrize("failure", ["signature", "async-start"]) +async def test_executor_start_failure_finishes_child_and_releases_budget(tmp_path, monkeypatch, failure): + (tmp_path / "authored.py").write_text("async def main(ctx, args):\n return 1\n") + environment = LocalEnvironment(str(tmp_path)) + parent, tool, runtime = _manager(environment) + native_exec = environment.exec_cmd + + if failure == "signature": + def fail_to_start(command, timeout=120): + raise AssertionError("the input_bytes argument must fail during binding") + else: + async def fail_to_start(command, timeout=120, *, input_bytes=None): + raise OSError("executor startup failed") + + monkeypatch.setattr(environment, "exec_cmd", fail_to_start) + try: + child = await _launch(tool, runtime) + assert tool._reserved_tokens() == 500 + result = json.loads(await tool.execute_with_runtime({"action": "wait", "id": child["id"]}, runtime)) + assert result["status"] == "failed" + assert ("TypeError" if failure == "signature" else "executor startup failed") in result["result"]["reason"] + assert tool._reserved_tokens() == 0 + owned = tool._children[child["id"]] + assert owned.task.done() and owned.task.exception() is None + assert owned.bridge._pool._closed + assert owned.cleanup_error is None + assert not environment._processes._processes + finally: + monkeypatch.setattr(environment, "exec_cmd", native_exec) + try: + await parent.aclose() + finally: + await environment.cleanup() + + +async def test_executor_cleanup_failure_retains_recovery_error(tmp_path, monkeypatch): + (tmp_path / "authored.py").write_text("async def main(ctx, args):\n return 1\n") + environment = LocalEnvironment(str(tmp_path)) + parent, tool, runtime = _manager(environment) + native_exec = environment.exec_cmd + + async def cleanup_failed(command, timeout=120, *, input_bytes=None): + raise ProcessCleanupError("process group still alive") + + monkeypatch.setattr(environment, "exec_cmd", cleanup_failed) + try: + child = await _launch(tool, runtime) + result = json.loads(await tool.execute_with_runtime({"action": "wait", "id": child["id"]}, runtime)) + assert result["status"] == "failed" + assert "process group still alive" in result["result"]["reason"] + owned = tool._children[child["id"]] + assert isinstance(owned.cleanup_error, ProcessCleanupError) + assert tool._reserved_tokens() == 0 + with pytest.raises(RuntimeError, match="process group still alive"): + await parent.aclose() + assert not owned.artifacts_removed + finally: + monkeypatch.setattr(environment, "exec_cmd", native_exec) + await environment.cleanup() diff --git a/tests/interfaces/test_cli_queued_focus.py b/tests/interfaces/test_cli_queued_focus.py index 84b1c32a..394182fa 100644 --- a/tests/interfaces/test_cli_queued_focus.py +++ b/tests/interfaces/test_cli_queued_focus.py @@ -235,3 +235,108 @@ async def respond(request): assert tui.selected_aid == 1 assert output.getvalue().count("Final lead answer after inspection") == 1 + +@pytest.mark.asyncio +@pytest.mark.parametrize(("question_count", "interrupted"), [(2, True), (1, True), (2, False)]) +async def test_question_batch_releases_the_input_after_interrupt( + monkeypatch, tmp_path, question_count, interrupted, +): + tui, output, team, cfg = _install_cli(monkeypatch, tmp_path) + team.write_text(team.read_text().replace( + "tools: [spawn_agent]", "tools: [spawn_agent, ask_user, file_write]", + )) + entered, release = asyncio.Event(), asyncio.Event() + requests = [] + + def tool(name, arguments, call_id): + return {"id": call_id, "type": "function", "function": { + "name": name, "arguments": json.dumps(arguments), + }} + + async def respond(request): + body = json.loads(request.content) + requests.append(body) + if "OFFLINE_ANALYST_ROLE" in body["messages"][0]["content"]: + return completion_http_response(request, text="Analyst completed") + user = next(message["content"] for message in reversed(body["messages"]) if message["role"] == "user") + if body["messages"][-1]["role"] != "user": + return completion_http_response(request, text="Lead completed") + if "setup_team" in user: + calls = [tool("spawn_agent", {"role": "analyst", "task": "initial_child_task"}, "spawn")] + elif "ask_batch" in user: + entered.set() + await release.wait() + calls = [tool("ask_user", {"question": f"Question {index}?"}, f"question_{index}") + for index in range(question_count)] + calls.append(tool("file_write", { + "path": "batch-completed.txt", "mode": "create", "content": "Batch completed", + }, "write")) + else: + return completion_http_response(request, text="Lead completed") + return http_response(request, 200, json={ + "id": "chatcmpl_questions", "object": "chat.completion", "created": 1, "model": body["model"], + "choices": [{"index": 0, "finish_reason": "tool_calls", "message": { + "role": "assistant", "content": None, "tool_calls": calls, + }}], + "usage": {"prompt_tokens": 20, "completion_tokens": 10, "total_tokens": 30}, + }) + + install_sdk_transport(monkeypatch, "openai", respond) + with create_pipe_input() as keyboard: + session = PromptSession(input=keyboard, output=DummyOutput(), erase_when_done=True) + _pin_status_row_under_input(session) + def prompt_session(prompt): + prompt._session = session + return session + + monkeypatch.setattr(LivePrompt, "_ensure_session", prompt_session) + run = asyncio.create_task(cli_main._run( + str(tmp_path), cfg, None, False, True, False, team_config_path=str(team), + )) + try: + await _wait_for(lambda: session.app.is_running) + keyboard.send_text("setup_team\r") + await _wait_for(lambda: _snapshot(tmp_path, 0) + and _snapshot(tmp_path, 0)["session_state"]["phase"] == "done") + keyboard.send_text("ask_batch\r") + await asyncio.wait_for(entered.wait(), timeout=5) + draft = "Continue the draft café" + keyboard.send_text(draft) + await _wait_for(lambda: session.default_buffer.text == draft) + release.set() + await _wait_for(lambda: "Question 0?" in "".join(text for _, text in session.message())) + keyboard.send_text("\t") + await _wait_for(lambda: tui.selected_aid == 1) + if interrupted: + keyboard.send_bytes(b"\x03") + await _wait_for(lambda: "Question 1?" in "".join(text for _, text in session.message()) + or _snapshot(tmp_path, 0)["session_state"]["phase"] == "stopped") + assert "Question 1?" not in "".join(text for _, text in session.message()) + else: + keyboard.send_text("/help\r") + await _wait_for(lambda: "Question 1?" in "".join(text for _, text in session.message())) + keyboard.send_text("src/\u7b2c\u4e8c\u4e2a.py\r") + phase = "stopped" if interrupted else "done" + await _wait_for(lambda: _snapshot(tmp_path, 0)["session_state"]["phase"] == phase) + await _wait_for(lambda: session.app.is_running and session.default_buffer.text == draft) + keyboard.send_text("\r") + await _wait_for(lambda: _snapshot(tmp_path, 1)["session_state"]["phase"] == "done" + and any(draft in (message.get("content") or "") + for message in _snapshot(tmp_path, 1)["messages"])) + keyboard.send_text("/exit\r") + await asyncio.wait_for(run, timeout=5) + finally: + release.set() + if not run.done(): + run.cancel() + await asyncio.gather(run, return_exceptions=True) + + messages = _snapshot(tmp_path, 0)["messages"] + answers = {message["tool_call_id"]: message["content"] for message in messages + if message["role"] == "tool" and message["tool_call_id"].startswith("question_")} + assert answers == ({f"question_{index}": "User declined to answer." for index in range(question_count)} + if interrupted else {"question_0": "/help", "question_1": "src/\u7b2c\u4e8c\u4e2a.py"}) + assert (tmp_path / "batch-completed.txt").read_text() == "Batch completed" + assert "Interactive team help" not in output.getvalue() + assert sum("OFFLINE_ANALYST_ROLE" in body["messages"][0]["content"] + and draft in body["messages"][-1]["content"] for body in requests) == 1 diff --git a/tests/interfaces/test_cli_weave_continuation.py b/tests/interfaces/test_cli_weave_continuation.py new file mode 100644 index 00000000..75e8e1e2 --- /dev/null +++ b/tests/interfaces/test_cli_weave_continuation.py @@ -0,0 +1,70 @@ +"""The CLI can retain the identity of an exported Weave run.""" + +from __future__ import annotations + +import asyncio +import json +import shlex +import sys + +import pytest +from typer.testing import CliRunner + +from opencollab import OpenCollab +from opencollab.adapters.cli.workflow import app +from tests.support.provider_sdk_http import completion_http_response, install_sdk_transport + + +@pytest.mark.parametrize("changed", [None, "run_id", "groups"]) +async def test_cli_continues_exported_weave_state_with_original_identity(tmp_path, monkeypatch, changed): + (tmp_path / "check.py").write_text( + "from pathlib import Path\n" + "assert 6 * 7 == 42\n" + "with Path('checks.log').open('a') as stream: stream.write('checked\\n')\n" + "print('CHECK_OK')\n", + encoding="utf-8", + ) + calls = [] + install_sdk_transport(monkeypatch, "openai", lambda request: ( + calls.append(request) or completion_http_response(request, text="ready"))) + args = {"groups": [{"id": "file", "prompt": "The fixture is ready"}], + "check_commands": [{"command": f"{shlex.quote(sys.executable)} check.py", "expected_output": "CHECK_OK"}], + "config": {"max_steps": 4, "max_repair_rounds": 0}} + first = await OpenCollab( + tmp_path, model="offline", provider="openai", api_key="offline-unused", # pragma: allowlist secret + base_url="https://offline.invalid/v1", config={"max_output_tokens": 128}, + ).workflow("weave", args, trace=False, budget=20_000, limit_mode="explicit") + assert first.ok and first.output["delivery_ok"] + args["state"] = json.loads(json.dumps(first.output["state"])) + run_id = args["state"]["run_id"] + if changed == "run_id": + run_id += "-other" + elif changed == "groups": + args["groups"][0]["prompt"] = "Different work" + calls_before = len(calls) + checks_before = (tmp_path / "checks.log").read_text().count("checked\n") + result = await asyncio.to_thread(lambda: CliRunner().invoke(app, [ + "run", "weave", "--workspace", str(tmp_path), "--args", json.dumps(args), + "--run-id", run_id, "--model", "offline", "--provider", "openai", + "--api-key", "offline-unused", "--base-url", "https://offline.invalid/v1", + "--budget", "20000", "--no-trace", + ])) + assert len(calls) == calls_before + if changed is None: + assert result.exit_code == 0, (result.stdout, result.exception) + else: + assert isinstance(result.exception, ValueError), (result.stdout, result.exception) + manifest_path, = (tmp_path / ".opencollab/sessions").glob("*/workflow.json") + manifest = json.loads(manifest_path.read_text()) + assert manifest["run_id"] == run_id + if changed is not None: + assert result.exit_code != 0 + assert isinstance(result.exception, ValueError) + assert "continuation requires" in str(result.exception) + assert manifest["status"] == "failed" + assert (tmp_path / "checks.log").read_text().count("checked\n") == checks_before + else: + assert result.exit_code == 0, (result.stdout, result.exception) + assert '"delivery_ok": true' in result.stdout + assert manifest["status"] == "completed" + assert (tmp_path / "checks.log").read_text().count("checked\n") > checks_before diff --git a/tests/interfaces/test_member_authored_workflow.py b/tests/interfaces/test_member_authored_workflow.py index 330ce25c..c74f0b57 100644 --- a/tests/interfaces/test_member_authored_workflow.py +++ b/tests/interfaces/test_member_authored_workflow.py @@ -266,12 +266,12 @@ async def test_stop_grace_leaves_finished_runner_result_persistence_owned( environment = local_environment_factory(str(tmp_path)) parent, tool, runtime = _manager(environment) entered, finish = asyncio.Event(), asyncio.Event() - native_read = environment.read_file + native_copy = authored_workflow._copy_artifact_to_host read_cancelled = False - async def delayed_result(path): + async def delayed_result(environment, path, target): nonlocal read_cancelled - value = await native_read(path) + await native_copy(environment, path, target) if path.endswith("/result.json"): entered.set() try: @@ -279,9 +279,8 @@ async def delayed_result(path): except asyncio.CancelledError: read_cancelled = True raise - return value - monkeypatch.setattr(environment, "read_file", delayed_result) + monkeypatch.setattr(authored_workflow, "_copy_artifact_to_host", delayed_result) child = await _launch(tool, runtime) stopping = None try: diff --git a/tests/interfaces/test_sdk_workflow_collection_results.py b/tests/interfaces/test_sdk_workflow_collection_results.py new file mode 100644 index 00000000..6fe10603 --- /dev/null +++ b/tests/interfaces/test_sdk_workflow_collection_results.py @@ -0,0 +1,52 @@ +import asyncio +import json + +import pytest + +from opencollab import OpenCollab + + +@pytest.mark.asyncio +@pytest.mark.parametrize("kind", ["parallel", "pipeline"]) +@pytest.mark.parametrize("capacity", [1, 2, 4]) +async def test_collections_preserve_handled_business_errors(tmp_path, kind, capacity): + inputs = [tmp_path / f"input-{index}.txt" for index in range(3)] + for path, value in zip(inputs, ["42", "invalid", "0"]): + path.write_text(value) + caught = [] + + async def parse(index): + text = await asyncio.to_thread(inputs[index].read_text) + try: + value = int(text) + except ValueError as exc: + caught.append(exc) + return exc + await asyncio.to_thread((tmp_path / f"result-{index}.txt").write_text, str(value)) + return value + + async def flow(ctx, _inputs): + if kind == "parallel": + values = await ctx.parallel([lambda index=index: parse(index) for index in range(3)]) + else: + async def stage(_previous, _item, index): + return await parse(index) + + values = await ctx.pipeline(list(range(3)), stage) + assert values[0] == 42 and values[1] is caught[0] and values[2] == 0 + summary = [str(value) if isinstance(value, Exception) else value for value in values] + await asyncio.to_thread((tmp_path / "summary.json").write_text, json.dumps(summary)) + assert await ctx.parallel([lambda: asyncio.sleep(0, result="recovered")]) == ["recovered"] + return summary + + artifacts = tmp_path / "artifacts" + result = await OpenCollab(tmp_path, model="test-model").workflow( + flow, concurrency=capacity, task_concurrency=capacity, timeout=10, + trace=False, artifacts=artifacts, + ) + assert result.status == "completed" + assert result.output == [42, str(caught[0]), 0] + assert json.loads((tmp_path / "summary.json").read_text()) == result.output + assert (tmp_path / "result-0.txt").read_text() == "42" + assert (tmp_path / "result-2.txt").read_text() == "0" + assert json.loads((artifacts / "workflow.json").read_text())["status"] == "completed" diff --git a/tests/runtime/test_anthropic_history_roundtrip.py b/tests/runtime/test_anthropic_history_roundtrip.py index bb2faf69..91e8cdea 100644 --- a/tests/runtime/test_anthropic_history_roundtrip.py +++ b/tests/runtime/test_anthropic_history_roundtrip.py @@ -6,7 +6,10 @@ from opencollab.adapters.llm.anthropic_provider import convert_to_anthropic_messages from opencollab.adapters.llm.client import LLMClient -from tests.support.provider_sdk_http import http_response, install_sdk_transport +from opencollab.bootstrap import build_session, load_session +from opencollab.domain.agent import Agent +from tests.support.environment_test_support import local_environment_factory as local_environment_factory +from tests.support.provider_sdk_http import completion_http_response, http_response, install_sdk_transport @pytest.mark.parametrize("history_shape", ["null", "omitted", "provider_state"]) @@ -53,7 +56,86 @@ async def handler(request): {"type": "tool_result", "tool_use_id": "toolu_lookup", "content": "42"} ] -@pytest.mark.parametrize("role,content", [("user", None), ("assistant", {"text": "invalid"})]) -def test_anthropic_history_retains_content_validation(role, content): - with pytest.raises(ValueError, match="content must be text or a content block list"): +@pytest.mark.parametrize("source_provider", ["anthropic", "responses"]) +@pytest.mark.parametrize("content", [ + "Keep answers concise and include the unit.", + [{"type": "text", "text": "Keep answers concise and include the unit."}], +]) +async def test_developer_session_restores_into_native_anthropic_request( + local_environment_factory, monkeypatch, tmp_path, source_provider, content, +): + monkeypatch.setenv("OPENCOLLAB_API_USAGE_LOG", str(tmp_path / "usage.jsonl")) + monkeypatch.setenv("OPENCOLLAB_BUDGET_NUDGE_MODE", "off") + requests = [] + + def handler(request): + requests.append(json.loads(request.content)) + return completion_http_response(request, text=f"answer {len(requests)}") + + install_sdk_transport(monkeypatch, "anthropic", handler) + if source_provider == "responses": + install_sdk_transport(monkeypatch, "openai", handler) + source_model = "gpt-5" if source_provider == "responses" else "claude-sonnet-4-6" + source_protocol = "responses" if source_provider == "responses" else "chat_completions" + source_agent = Agent(name="lead", system_prompt="Answer distance questions.", model=source_model, + provider="openai" if source_provider == "responses" else "anthropic", + wire_protocol=source_protocol) + path = tmp_path / "session.json" + async with LLMClient( + model=source_model, provider=source_agent.provider, wire_protocol=source_protocol, + api_key="test-placeholder", base_url="https://provider.invalid", max_retries=0, # pragma: allowlist secret + ) as source_client: + source = build_session(agent=source_agent, llm=source_client, env=local_environment_factory()) + try: + source.messages = [*source.messages, {"role": "developer", "content": content}] + await source.add_user_message("How far is twelve kilometres?") + assert await source.run_loop() == "answer 1" + source.save(str(path)) + saved_messages = source.messages.copy() + finally: + await source.aclose() + + agent = Agent(name="lead", system_prompt=source_agent.system_prompt, + model="claude-sonnet-4-6", provider="anthropic") + async with LLMClient( + model=agent.model, provider="anthropic", api_key="test-placeholder", # pragma: allowlist secret + base_url="https://provider.invalid", max_retries=0, + ) as client: + restored = load_session(str(path), agent=agent, llm=client, env=local_environment_factory()) + try: + assert restored.messages == saved_messages + await restored.add_user_message("Repeat that distance briefly.") + assert await restored.run_loop() == "answer 2" + assert restored.messages[:len(saved_messages)] == saved_messages + finally: + await restored.aclose() + + assert len(requests) == 2 + assert requests[1]["system"] == "Answer distance questions.\n\nKeep answers concise and include the unit." + assert [message["role"] for message in requests[1]["messages"]] == ["user", "assistant", "user"] + assert requests[1]["messages"][-1]["content"] == "Repeat that distance briefly." + + +def test_anthropic_developer_summary_keeps_its_conversation_position(): + system, messages = convert_to_anthropic_messages([ + {"role": "system", "content": "Answer distance questions."}, + {"role": "developer", "content": [{"type": "input_text", "text": "Keep units."}]}, + {"role": "user", "content": "Earlier question."}, + {"role": "assistant", "content": "Earlier answer."}, + {"role": "developer", "content": "The distance is twelve kilometres.", "compacted": True}, + {"role": "user", "content": "Repeat the distance."}, + ]) + assert system == ["Answer distance questions.", "Keep units."] + assert messages[2] == {"role": "user", "content": "The distance is twelve kilometres."} + assert messages[3] == {"role": "user", "content": "Repeat the distance."} + + +@pytest.mark.parametrize("role,content,error", [ + ("user", None, "content must be text or a content block list"), + ("assistant", {"text": "invalid"}, "content must be text or a content block list"), + ("developer", None, "content must be text or a content block list"), + ("developer", [{"type": "text", "text": 42}], "requires text"), +]) +def test_anthropic_history_retains_content_validation(role, content, error): + with pytest.raises(ValueError, match=error): convert_to_anthropic_messages([{"role": role, "content": content}]) diff --git a/tests/runtime/test_authored_artifact_transfer.py b/tests/runtime/test_authored_artifact_transfer.py new file mode 100644 index 00000000..2fa7d6c6 --- /dev/null +++ b/tests/runtime/test_authored_artifact_transfer.py @@ -0,0 +1,171 @@ +from __future__ import annotations + +import asyncio +import os +import shutil +from pathlib import Path +from unittest.mock import AsyncMock + +import pytest + +from opencollab.adapters import _docker_snapshot as snapshot_module +from opencollab.adapters._docker_snapshot import _copy_artifact_to_host +from opencollab.adapters._env_vm import VMEnvironment +from opencollab.adapters.env import DockerEnvironment, ExecResult, LocalEnvironment, WorktreeEnvironment +from opencollab.adapters.session_environments import SessionEnvironmentPool +from opencollab.application.tool_execution import ToolRuntime +from opencollab.bootstrap.authored_workflow_bridge import AuthoredWorkflowBridge, BridgeClient, BridgeEnvironment + + +@pytest.mark.parametrize("kind", ["local", "worktree"]) +@pytest.mark.parametrize("target_kind", ["relative-escape", "absolute-escape", "file-link", "directory-link", "fifo"]) +async def test_artifact_transfer_preserves_local_path_and_regular_file_checks(tmp_path, kind, target_kind): + workspace = tmp_path / "workspace" + workspace.mkdir() + environment = (LocalEnvironment if kind == "local" else WorktreeEnvironment)(str(workspace)) + await environment.setup() + root = Path(environment.workspace) + outside = root.parent / f"{root.name}-outside" + outside.mkdir() + (outside / "result.json").write_text('{"outside": true}') + if target_kind == "relative-escape": + source = f"../{outside.name}/result.json" + elif target_kind == "absolute-escape": + source = str(outside / "result.json") + elif target_kind == "file-link": + (root / "result.json").symlink_to(outside / "result.json") + source = str(root / "result.json") + elif target_kind == "directory-link": + (root / "artifacts").symlink_to(outside, target_is_directory=True) + source = str(root / "artifacts" / "result.json") + else: + os.mkfifo(root / "result.json") + source = str(root / "result.json") + target = tmp_path / "copied.json" + try: + with pytest.raises((OSError, PermissionError)): + await _copy_artifact_to_host(environment, source, target) + assert not target.exists() + finally: + for name in ("result.json", "artifacts"): + if os.path.lexists(root / name): + (root / name).unlink() + shutil.rmtree(outside) + if isinstance(environment, WorktreeEnvironment): + await environment.get_diff() + await environment.cleanup() + + +async def test_artifact_transfer_rejects_replaced_workspace(tmp_path): + workspace = tmp_path / "workspace" + workspace.mkdir() + environment = LocalEnvironment(str(workspace)) + workspace.rename(tmp_path / "original") + workspace.mkdir() + (workspace / "result.json").write_text('{"replacement": true}') + try: + with pytest.raises(OSError, match="workspace was replaced"): + await _copy_artifact_to_host(environment, str(workspace / "result.json"), tmp_path / "copied.json") + assert not (tmp_path / "copied.json").exists() + finally: + await environment.cleanup() + + +@pytest.mark.parametrize("kind", ["docker", "vm"]) +@pytest.mark.parametrize("symlink", [False, True]) +async def test_remote_artifact_transfer_uses_existing_transport_and_requires_regular_output( + tmp_path, monkeypatch, kind, symlink, +): + environment = object.__new__(DockerEnvironment if kind == "docker" else VMEnvironment) + environment.workspace = "/workspace" + target = tmp_path / "copied.json" + native = tmp_path / "native.json" + native.write_bytes(b'{"payload":"' + b"x" * (4 * 1024 * 1024) + b'"}\n') + + async def copy(destination): + if symlink: + destination.symlink_to(native) + else: + shutil.copyfile(native, destination) + + if kind == "docker": + environment._container_id = "owned-test-container" + monkeypatch.setattr(environment, "setup", AsyncMock()) + contained = AsyncMock(return_value="/workspace/physical/result.json") + monkeypatch.setattr(environment, "_contained_file_path", contained) + + async def docker(*args): + assert args[:3] == ("cp", "--", "owned-test-container:/workspace/physical/result.json") + await copy(Path(args[3])) + return ExecResult(0, "", "") + + monkeypatch.setattr(environment, "_docker", docker) + else: + async def download(source, destination, *, directory=False): + assert source == "/workspace/result.json" and not directory + await copy(destination) + + monkeypatch.setattr(environment, "copy_to_host", download) + if symlink: + with pytest.raises(OSError, match="regular file"): + await _copy_artifact_to_host(environment, "/workspace/result.json", target) + else: + await _copy_artifact_to_host(environment, "/workspace/result.json", target) + assert target.read_bytes() == native.read_bytes() + if kind == "docker": + contained.assert_awaited_once_with("/workspace/result.json") + with pytest.raises(PermissionError, match="escapes workspace"): + await _copy_artifact_to_host(environment, "/outside/result.json", target) + + +async def test_artifact_copy_failure_closes_source_descriptor(tmp_path, monkeypatch): + (tmp_path / "result.json").write_text('{"ready": true}') + environment = LocalEnvironment(str(tmp_path)) + descriptor = None + + def fail_copy(source, target): + nonlocal descriptor + descriptor = source.fileno() + raise OSError("destination write failed") + + monkeypatch.setattr(snapshot_module.shutil, "copyfileobj", fail_copy) + try: + with pytest.raises(OSError, match="destination write failed"): + await _copy_artifact_to_host(environment, "result.json", tmp_path / "copied.json") + assert descriptor is not None + with pytest.raises(OSError): + os.fstat(descriptor) + finally: + await environment.cleanup() + + +async def test_artifact_transfer_retains_bridge_environment_fallback(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + artifacts = tmp_path / "bridge-artifacts" + artifacts.mkdir() + (tmp_path / "result.json").write_text('{"ready": true}') + environment = LocalEnvironment(str(tmp_path)) + bridge = AuthoredWorkflowBridge( + environment, str(artifacts), + SessionEnvironmentPool(str(tmp_path), isolation="shared", base_environment=environment), + {}, ToolRuntime(environment, None, None), allow_unisolated_shell=True, + ) + proxy = BridgeEnvironment(BridgeClient(artifacts, bridge.options()), { + "handle": 0, "workspace": str(tmp_path), "process_isolated": False, + }) + copying = asyncio.create_task(_copy_artifact_to_host(proxy, "result.json", tmp_path / "copied.json")) + + async def poll_until_copied(): + while not copying.done(): + await bridge.poll() + await asyncio.sleep(0.01) + + try: + await asyncio.wait_for(poll_until_copied(), 5) + await copying + assert (tmp_path / "copied.json").read_text() == (tmp_path / "result.json").read_text() + finally: + copying.cancel() + await asyncio.gather(copying, return_exceptions=True) + await bridge.aclose() + await environment.cleanup() diff --git a/tests/runtime/test_bounded_summary.py b/tests/runtime/test_bounded_summary.py index 4e27ac24..47febca9 100644 --- a/tests/runtime/test_bounded_summary.py +++ b/tests/runtime/test_bounded_summary.py @@ -113,7 +113,10 @@ async def complete(request): assert requests == [build_summary_request(segment)] assert summarizer.last_call_cacheable -@pytest.mark.parametrize("failure", [RuntimeError("provider failed"), " "]) +@pytest.mark.parametrize("failure", [ + RuntimeError("provider failed"), " ", + SimpleNamespace(content="Truncated earlier changes.", finish_reason="max_tokens"), +]) async def test_chunk_failure_stops_before_the_tail_and_never_returns_a_partial_summary(failure): requests = [] @@ -123,7 +126,7 @@ async def complete(request): return SimpleNamespace(content="earlier user changes") if isinstance(failure, Exception): raise failure - return SimpleNamespace(content=failure) + return SimpleNamespace(content=failure) if isinstance(failure, str) else failure summarizer = ReadTimeSummarizer( complete, context_window=WINDOW, max_output_tokens=OUTPUT, transcript_path="session.json", diff --git a/tests/runtime/test_compaction_summary.py b/tests/runtime/test_compaction_summary.py index 0dcaf249..4c650536 100644 --- a/tests/runtime/test_compaction_summary.py +++ b/tests/runtime/test_compaction_summary.py @@ -5,6 +5,8 @@ import asyncio from types import SimpleNamespace +import pytest + import opencollab.application.compaction_summary as compaction_summary from opencollab.application.compaction_summary import ReadTimeSummarizer, run_coro_blocking @@ -68,6 +70,24 @@ def test_summarizer_falls_back_on_llm_error(): assert "Summary:" not in out assert s.last_call_cacheable is False +@pytest.mark.parametrize("finish_reason", ["length", "max_tokens", "model_context_window_exceeded"]) +async def test_truncation_rejects_even_a_well_formed_summary(finish_reason): + async def complete(_request): + return SimpleNamespace(content="Partial context.", finish_reason=finish_reason) + + summarizer = ReadTimeSummarizer(complete) + assert await summarizer.asummarize(SEGMENT) == "[user]: build the thing\n[assistant]: working on it" + assert not summarizer.last_call_cacheable + +@pytest.mark.parametrize("finish_reason", [None, "stop", "end_turn", "stop_sequence"]) +async def test_complete_summary_accepts_supported_stop_reasons(finish_reason): + async def complete(_request): + return SimpleNamespace(content="Complete context.", finish_reason=finish_reason) + + summarizer = ReadTimeSummarizer(complete) + assert await summarizer.asummarize(SEGMENT) == "Summary:\nComplete context." + assert summarizer.last_call_cacheable + def test_summarizer_falls_back_when_no_summary_block(): s = _summarizer("only thinking, forgot the summary") out = s(SEGMENT) diff --git a/tests/runtime/test_developer_session_persistence.py b/tests/runtime/test_developer_session_persistence.py new file mode 100644 index 00000000..d7abb53d --- /dev/null +++ b/tests/runtime/test_developer_session_persistence.py @@ -0,0 +1,159 @@ +from __future__ import annotations + +import json + +import pytest + +from opencollab.adapters.llm.client import LLMClient +from opencollab.adapters.storage import SessionStore +from opencollab.bootstrap import build_session, load_session +from opencollab.domain.agent import Agent +from opencollab.domain.session import SessionPhase +from opencollab.sdk import OpenCollab +from tests.support.environment_test_support import local_environment_factory as local_environment_factory +from tests.support.provider_sdk_http import http_response, install_sdk_transport +from tests.support.responses_provider_test_support import message_item + + +@pytest.mark.parametrize("content", ["Use a concise answer.", [{"type": "text", "text": "Use a concise answer."}]]) +@pytest.mark.parametrize("saved", ["autosave", "checkpoint", "export"]) +async def test_responses_developer_history_survives_save_and_next_turn( + local_environment_factory, monkeypatch, tmp_path, content, saved, +): + usage_path = tmp_path / "usage.jsonl" + monkeypatch.setenv("OPENCOLLAB_API_USAGE_LOG", str(usage_path)) + monkeypatch.setenv("OPENCOLLAB_BUDGET_NUDGE_MODE", "off") + requests = [] + + def handler(request): + body = json.loads(request.content) + requests.append(body) + return http_response(request, 200, json={ + "id": f"resp_{len(requests)}", "object": "response", "created_at": 1, + "status": "completed", "error": None, "incomplete_details": None, + "model": body["model"], "output": [message_item(f"answer {len(requests)}")], + "usage": {"input_tokens": 10, "output_tokens": 2, "total_tokens": 12}, + }) + + install_sdk_transport(monkeypatch, "openai", handler) + automatic = tmp_path / "automatic.json" + path = tmp_path / "exported.json" if saved == "export" else automatic + agent = Agent(name="lead", system_prompt="System instructions.", model="o1-pro", wire_protocol="responses") + async with LLMClient( + model="o1-pro", wire_protocol="responses", max_retries=0, + api_key="controlled-test", # pragma: allowlist secret + base_url="https://controlled.invalid/v1", + ) as client: + original = build_session( + agent=agent, env=local_environment_factory(), llm=client, + auto_save_path=str(automatic), aid=7, run_id="developer-roundtrip", + ) + restored = None + try: + original.messages = [*original.messages, {"role": "developer", "content": content}] + await original.add_user_message("First request.") + assert await original.run_loop() == "answer 1" + assert original.persistence_errors == () + assert original.phase is SessionPhase.DONE + assert original.used_tokens == 12 + if saved != "autosave": + original.save(str(path)) + snapshot = OpenCollab.read_session_snapshot(path) + assert snapshot["messages"][1]["role"] == "developer" + assert snapshot["messages"][1]["content"] == content + assert snapshot["aid"] == 7 + assert snapshot["run_id"] == "developer-roundtrip" + assert snapshot["session_state"]["used_tokens"] == 12 + assert snapshot["session_state"]["phase"] == "done" + messages = original.messages.copy() + await original.aclose() + + restored = load_session( + str(path), agent=agent, env=local_environment_factory(), llm=client, + auto_save_path=str(automatic), + ) + assert restored.messages == messages + assert restored.state.aid == 7 + assert restored.run_id == "developer-roundtrip" + assert restored.used_tokens == 12 + assert restored.phase is SessionPhase.DONE + await restored.add_user_message("Second request.") + assert await restored.run_loop() == "answer 2" + assert restored.used_tokens == 24 + assert restored.persistence_errors == () + persisted = OpenCollab.read_session_snapshot(automatic) + assert persisted["session_state"]["used_tokens"] == 24 + assert persisted["aid"] == 7 + assert persisted["run_id"] == "developer-roundtrip" + assert persisted["messages"][1]["role"] == "developer" + assert persisted["messages"][1]["content"] == content + finally: + await original.aclose() + if restored is not None: + await restored.aclose() + + assert len(requests) == 2 + expected = content if isinstance(content, str) else [{"type": "input_text", "text": content[0]["text"]}] + for request in requests: + assert request["instructions"] == "System instructions." + assert request["input"][0] == {"role": "developer", "content": expected} + assert requests[0]["input"][1] == {"role": "user", "content": "First request."} + assert requests[1]["input"][-1] == {"role": "user", "content": "Second request."} + assert [row["role"] for row in requests[1]["input"]] == ["developer", "user", "assistant", "user"] + usage = [json.loads(line) for line in usage_path.read_text(encoding="utf-8").splitlines()] + assert len(usage) == 2 + assert [row["usage"]["total_tokens"] for row in usage] == [12, 12] + + +@pytest.mark.parametrize("format", ["json", "legacy-json", "jsonl", "journal", "checkpoint"]) +@pytest.mark.parametrize("role", ["developer", "system", "user"]) +def test_instruction_roles_round_trip_through_store_formats(tmp_path, format, role): + path = tmp_path / "session.json" + messages = [ + {"role": "system", "content": "System instructions."}, + {"role": role, "content": [{"type": "input_text", "text": "Keep this instruction."}]}, + ] + store = SessionStore() + if format == "json": + store.save(str(path), messages, meta={"snapshot_version": 1, "aid": 7}) + elif format == "legacy-json": + path.write_text(json.dumps(messages), encoding="utf-8") + elif format == "jsonl": + path.write_text("".join(json.dumps(message) + "\n" for message in messages), encoding="utf-8") + elif format == "journal": + store.append_snapshot_delta(str(path), sequence=1, replace_from=0, messages=messages, meta={}) + else: + store.append_snapshot_delta(str(path), sequence=1, replace_from=0, messages=messages, meta={}) + store.checkpoint_snapshot(str(path), messages, sequence=1, meta={}) + assert store.load_messages(str(path), "fallback") == messages + + +@pytest.mark.parametrize("format", ["json", "jsonl", "journal"]) +@pytest.mark.parametrize(("message", "error"), [ + ({"role": "developer"}, "missing 'content'"), + ({"role": "developer", "content": None}, "must be text or a content-part list"), + ({"role": "developer", "content": {}}, "must be text or a content-part list"), + ({"role": "developer", "content": []}, "part list must not be empty"), + ({"role": "developer", "content": [{"type": "input_text"}]}, "requires text"), + ({"role": "developer", "content": [{"type": "text", "text": 42}]}, "requires text"), + ({"role": "unknown", "content": "invalid"}, "Invalid message role"), + ({"role": "Developer", "content": "invalid"}, "Invalid message role"), + ({"role": "tool", "content": "result"}, "expected non-empty 'tool_call_id'"), +]) +def test_store_still_rejects_malformed_messages(tmp_path, format, message, error): + path = tmp_path / "invalid.json" + store = SessionStore() + if format == "json": + path.write_text(json.dumps({"messages": [message]}), encoding="utf-8") + elif format == "jsonl": + path.write_text( + json.dumps({"role": "system", "content": "System instructions."}) + "\n" + json.dumps(message) + "\n", + encoding="utf-8", + ) + else: + with pytest.raises(ValueError, match=error): + store.append_snapshot_delta(str(path), sequence=1, replace_from=0, messages=[message], meta={}) + assert not store.has_snapshot(str(path)) + return + with pytest.raises(ValueError, match=error): + store.load_snapshot(str(path), "fallback") diff --git a/tests/runtime/test_disabled_sparse_checkout.py b/tests/runtime/test_disabled_sparse_checkout.py new file mode 100644 index 00000000..8c18e02b --- /dev/null +++ b/tests/runtime/test_disabled_sparse_checkout.py @@ -0,0 +1,75 @@ +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from opencollab.adapters.env import LocalEnvironment, WorktreeEnvironment +from opencollab.adapters.tools.git_diff import GitDiffTool +from opencollab.adapters.worktree_pool import WorktreePool +from opencollab.application.tool_execution import ToolRuntime + + +def _git(repo: Path, *args: str) -> str: + return subprocess.run( + ["git", "-C", str(repo), *args], check=True, text=True, capture_output=True, + ).stdout + + +@pytest.mark.parametrize("disabled_sparse", [False, True]) +async def test_full_checkout_exports_after_sparse_checkout_disable(tmp_path, monkeypatch, disabled_sparse): + monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "1") + monkeypatch.setenv("GIT_CONFIG_GLOBAL", "/dev/null") + source = tmp_path / "source" + source.mkdir() + for name, content in (("src/app.py", "value = 1\n"), ("docs/guide.txt", "guide\n")): + path = source / name + path.parent.mkdir(exist_ok=True) + path.write_text(content, encoding="utf-8") + _git(source, "init", "-q") + _git(source, "add", ".") + _git(source, "-c", "user.name=Test User", "-c", "user.email=test@example.invalid", + "-c", "commit.gpgsign=false", "commit", "-qm", "base") + if disabled_sparse: + _git(source, "sparse-checkout", "init", "--cone") + _git(source, "sparse-checkout", "set", "src") + assert not (source / "docs/guide.txt").exists() + _git(source, "sparse-checkout", "disable") + for key in ("core.sparsecheckout", "core.sparsecheckoutcone"): + assert _git(source, "config", "--type=bool", "--get", key) == "false\n" + assert (source / "docs/guide.txt").read_text(encoding="utf-8") == "guide\n" + assert _git(source, "status", "--porcelain") == "" + assert all(line.startswith("H ") for line in _git(source, "ls-files", "-t").splitlines()) + + local = LocalEnvironment(str(source)) + try: + result = await GitDiffTool().execute_with_runtime({}, ToolRuntime(local, None, None)) + assert result == "Status: working tree clean.\n\ndiff vs HEAD: (no changes)" + finally: + await local.cleanup() + + env = WorktreeEnvironment(str(source)) + candidate = Path(await env.setup()) + try: + await env.write_file("src/app.py", "value = 2\n") + expected = _git(candidate, "diff", "--binary", "--full-index", "--no-color", + "--unified=3", "--src-prefix=a/", "--dst-prefix=b/", "HEAD") + assert "+value = 2\n" in expected + assert await env.get_diff() == expected + assert _git(candidate, "diff", "--cached") == "" + finally: + await env.cleanup() + assert not candidate.exists() + + pool = WorktreePool(str(source), use_worktrees=True, enable_adoption=True) + try: + worker = await pool.acquire("coder") + pooled_candidate = Path(worker.workspace) + await worker.write_file("src/app.py", "value = 2\n") + assert await worker.get_diff() == expected + finally: + await pool.release() + assert not pooled_candidate.exists() + assert _git(source, "status", "--porcelain") == "" + assert (source / "src/app.py").read_text(encoding="utf-8") == "value = 1\n" diff --git a/tests/runtime/test_llm_provider_contracts.py b/tests/runtime/test_llm_provider_contracts.py index 23cf92c8..813a837b 100644 --- a/tests/runtime/test_llm_provider_contracts.py +++ b/tests/runtime/test_llm_provider_contracts.py @@ -447,10 +447,10 @@ def test_responses_converts_image_url_instead_of_dropping_it(): ] def test_anthropic_rejects_unknown_message_role_instead_of_dropping_it(): - with pytest.raises(ValueError, match="unsupported role 'developer'"): + with pytest.raises(ValueError, match="unsupported role 'unknown'"): build_anthropic_kwargs( "claude-sonnet-5", - [{"role": "developer", "content": "hidden instruction"}], + [{"role": "unknown", "content": "hidden instruction"}], None, 0.2, ) diff --git a/tests/runtime/test_summary_completion_http.py b/tests/runtime/test_summary_completion_http.py new file mode 100644 index 00000000..20667243 --- /dev/null +++ b/tests/runtime/test_summary_completion_http.py @@ -0,0 +1,116 @@ +"""Provider truncation retains history until a complete summary can be cached.""" + +from __future__ import annotations + +import copy +import json + +import pytest + +from opencollab.adapters.env import LocalEnvironment +from opencollab.adapters.llm.client import LLMClient +from opencollab.bootstrap import build_session +from opencollab.domain.agent import Agent +from tests.support.provider_sdk_http import http_response, install_sdk_transport +from tests.support.responses_provider_test_support import message_item + +REQUIREMENT = "Preserve the SQLite backup before replacing the database." + + +@pytest.mark.parametrize("configuration", [ + {"provider": "openai", "wire_protocol": "chat_completions", "model": "gpt-4o"}, + {"provider": "openai", "wire_protocol": "responses", "model": "gpt-4o"}, + {"provider": "anthropic", "wire_protocol": "chat_completions", "model": "claude-sonnet-4-6"}, +], ids=["chat", "responses", "anthropic"]) +@pytest.mark.parametrize("truncated", [False, True], ids=["complete", "truncated"]) +async def test_native_summary_completion_preserves_history_usage_and_cache( + tmp_path, monkeypatch, configuration, truncated, +): + requests = [] + monkeypatch.setenv("OPENCOLLAB_API_USAGE_LOG", str(tmp_path / "usage.jsonl")) + monkeypatch.setenv("OPENCOLLAB_BUDGET_NUDGE_MODE", "off") + monkeypatch.setenv("OPENCOLLAB_WRITE_NUDGE_MODE", "off") + + async def handler(request): + body = json.loads(request.content) + requests.append(body) + summary = "create a detailed summary of the conversation" in json.dumps(body) + partial = summary and truncated and len(requests) == 1 + text = ( + "Earlier setup notes." if partial else f"{REQUIREMENT}" + ) if summary else ("Backup preserved." if REQUIREMENT in json.dumps(body) else "Backup missing.") + output_tokens = 1_024 if partial else 10 + if configuration["provider"] == "anthropic": + return http_response(request, 200, json={ + "id": "msg_summary", "type": "message", "role": "assistant", "model": body["model"], + "content": [{"type": "text", "text": text}], + "stop_reason": "max_tokens" if partial else "end_turn", "stop_sequence": None, + "usage": {"input_tokens": 100, "output_tokens": output_tokens}, + }) + if configuration["wire_protocol"] == "responses": + item = message_item(text) + item["status"] = "incomplete" if partial else "completed" + terminal = { + "id": "resp_summary", "object": "response", "model": body["model"], + "status": "incomplete" if partial else "completed", "error": None, + "incomplete_details": {"reason": "max_output_tokens"} if partial else None, + "output": [item], + "usage": {"input_tokens": 100, "output_tokens": output_tokens, "total_tokens": 100 + output_tokens}, + } + events = [ + {"type": "response.output_item.done", "output_index": 0, "item": item}, + {"type": "response.incomplete" if partial else "response.completed", "response": terminal}, + ] + wire = "".join(f"data: {json.dumps(event)}\n\n" for event in events) + return http_response(request, 200, headers={"content-type": "text/event-stream"}, text=wire) + return http_response(request, 200, json={ + "id": "chat_summary", "object": "chat.completion", "created": 1, "model": body["model"], + "choices": [{"index": 0, "message": {"role": "assistant", "content": text}, + "finish_reason": "length" if partial else "stop"}], + "usage": {"prompt_tokens": 100, "completion_tokens": output_tokens, "total_tokens": 100 + output_tokens}, + }) + + install_sdk_transport(monkeypatch, configuration["provider"], handler) + async with LLMClient( + **configuration, api_key="controlled-test", # pragma: allowlist secret + base_url="https://controlled.invalid/v1", max_retries=0, context_window=40_000, + ) as client: + session = build_session( + agent=Agent(name="notes", system_prompt="Complete the user task.", + **configuration, tools=[], max_tokens_per_step=1_024), + llm=client, env=LocalEnvironment(str(tmp_path)), max_budget_tokens=None, + ) + original = [ + {"role": "system", "content": "Complete the user task."}, + {"role": "user", "content": "Earlier setup notes. " * 1_800 + REQUIREMENT}, + {"role": "assistant", "content": "Earlier result."}, + {"role": "user", "content": "The next work item is ready."}, + {"role": "assistant", "content": "Another result."}, + {"role": "user", "content": "Now finish the task."}, + ] + session.messages = copy.deepcopy(original) + try: + assert await session.run_loop() == "Backup preserved." + assert len(requests) == 2 + assert REQUIREMENT in json.dumps(requests[0]) + assert REQUIREMENT in json.dumps(requests[1]) + assert session.messages[:len(original)] == original + assert session.used_tokens == (1_234 if truncated else 220) + + compact = session.runner.shaper._shapers[-1] + assert compact.summarizer.last_call_cacheable is (not truncated) + assert len(compact._summary_cache) == (0 if truncated else 1) + if truncated: + assert original[1]["content"] in json.dumps(requests[1]) + + projection = await session.runner.shaper.ashape(original) + assert REQUIREMENT in json.dumps(projection) + assert len(requests) == (3 if truncated else 2) + assert compact.summarizer.last_call_cacheable + assert len(compact._summary_cache) == 1 + assert await session.runner.shaper.ashape(original) == projection + assert len(requests) == (3 if truncated else 2) + assert session.used_tokens == (1_344 if truncated else 220) + finally: + await session.aclose() + await session.env.cleanup() diff --git a/tests/runtime/test_worktree_pool.py b/tests/runtime/test_worktree_pool.py index dac98e46..75b732d2 100644 --- a/tests/runtime/test_worktree_pool.py +++ b/tests/runtime/test_worktree_pool.py @@ -47,6 +47,20 @@ def _repo(path): def _branch_exists(repo, branch: str) -> bool: return _git(repo, "show-ref", "--verify", f"refs/heads/{branch}", check=False).returncode == 0 + +async def test_worktree_exec_forwards_binary_stdin(tmp_path): + environment = WorktreeEnvironment(str(_repo(tmp_path / "repo"))) + payload = b"\x00workflow\n\xff" + try: + result = await environment.exec_cmd(shlex.join([ + sys.executable, "-c", "import sys; print(sys.stdin.buffer.read().hex())", + ]), timeout=10, input_bytes=payload) + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == payload.hex() + finally: + await environment.cleanup() + + @pytest.mark.parametrize("suffix", ["", " ", "\n"]) @pytest.mark.parametrize("subdirectory", [False, True]) async def test_git_worktree_keeps_whitespace_in_repository_root( diff --git a/tests/tools/test_git_patch.py b/tests/tools/test_git_patch.py index b9e13aef..dc59339a 100644 --- a/tests/tools/test_git_patch.py +++ b/tests/tools/test_git_patch.py @@ -205,6 +205,59 @@ def test_patch_rejects_worktree_scoped_sparse_checkout(tmp_path): assert result.returncode == 125 assert "core.sparsecheckout" in result.stderr +@pytest.mark.parametrize("scope", ["--local", "--worktree"]) +@pytest.mark.parametrize("value", ["false", "FaLsE", "no", "off", "0", ""]) +def test_patch_allows_disabled_sparse_boolean_values(tmp_path, scope, value): + repo = _repo(tmp_path) + (repo / "tracked.txt").write_text("new\n", encoding="utf-8") + expected = _extract(repo) + assert expected.returncode == 0, expected.stderr + _git(repo, "config", "extensions.worktreeConfig", "true") + for key in ("core.sparsecheckout", "core.sparsecheckoutcone"): + _git(repo, "config", scope, key, value) + + result = _extract(repo) + + assert result.returncode == 0, result.stderr + assert result.stdout == expected.stdout + +@pytest.mark.parametrize("scope", ["--local", "--worktree"]) +@pytest.mark.parametrize("key", ["core.sparsecheckout", "core.sparsecheckoutcone"]) +@pytest.mark.parametrize("value", ["true", "yes", "on", "1", "-1", "2", "invalid", None]) +def test_patch_rejects_enabled_or_invalid_sparse_values(tmp_path, scope, key, value): + repo = _repo(tmp_path) + _git(repo, "config", "extensions.worktreeConfig", "true") + if value is None: + config = Path(_git(repo, "rev-parse", "--git-path", "config.worktree" if scope == "--worktree" + else "config").stdout.strip()) + with (repo / config).open("a", encoding="utf-8") as stream: + stream.write(f"[core]\n\t{key.split('.')[1]}\n") + else: + _git(repo, "config", scope, key, value) + + result = _extract(repo) + + assert result.returncode == 125 + assert f"unsafe repository Git configuration: {key}" in result.stderr + +@pytest.mark.parametrize("sparse_index", [False, True]) +def test_patch_rejects_active_sparse_checkout(tmp_path, sparse_index): + repo = _repo(tmp_path) + for directory in ("visible", "hidden"): + (repo / directory).mkdir() + (repo / directory / "file.txt").write_text("tracked\n", encoding="utf-8") + _git(repo, "add", ".") + _git(repo, "commit", "-qm", "add directories") + _git(repo, "sparse-checkout", "init", "--cone", "--sparse-index" if sparse_index else "--no-sparse-index") + _git(repo, "sparse-checkout", "set", "visible") + assert not (repo / "hidden" / "file.txt").exists() + assert "S hidden/" in _git(repo, "ls-files", "--sparse", "-t").stdout + + result = _extract(repo) + + assert result.returncode == 125 + assert "core.sparsecheckout" in result.stderr + def test_patch_rejects_transform_config_loaded_through_include(tmp_path): repo = _repo(tmp_path) (repo / "payload.txt").write_text("candidate\n", encoding="utf-8") diff --git a/tests/tools/test_native_bash_evidence.py b/tests/tools/test_native_bash_evidence.py index a7153744..29bb7cf5 100644 --- a/tests/tools/test_native_bash_evidence.py +++ b/tests/tools/test_native_bash_evidence.py @@ -48,6 +48,24 @@ def test_native_bash_schema_output_command_and_timeout_are_unchanged(): assert observed.verification_records[-1]["command"] == COMMAND assert "Verdict:" not in result +@pytest.mark.parametrize("command, expected", [ + ("python -m pytest tests.py -q", "python -m pytest tests.py -q -rA"), + ('python -m pytest -k "$SELECTION" "test file.py" -q', + 'python -m pytest -k "$SELECTION" "test file.py" -q -rA'), + ('python -m pytest -q -- "test file.py"', 'python -m pytest -q -rA -- "test file.py"'), + ('cd pytest && env -- python -m pytest -q -- test.py', + 'cd pytest && env -- python -m pytest -q -rA -- test.py'), + ('uv run -- python -m pytest -q test.py', 'uv run -- python -m pytest -q test.py -rA'), + ('echo Starting && python -m pytest -q test.py', 'echo Starting && python -m pytest -q test.py'), + ('python -m pytest test.py > report.txt', 'python -m pytest test.py > report.txt'), + ('python -m pytest test.py && echo finished', 'python -m pytest test.py && echo finished'), + ('python -m pytest --collect-only', 'python -m pytest --collect-only'), + ('python -m pytest --help', 'python -m pytest --help'), +]) +def test_opt_in_pass_reporting_preserves_shell_text_and_target_separator(command, expected): + tool = evidence_tools("bash")[0] + assert tool.with_pass_reporting(command) == expected + def test_removed_tool_is_unavailable(): with pytest.raises(ValueError, match="unsupported built-in"): evidence_tools("run_tests") diff --git a/tests/workflows/test_candidate_workflow_budget_inheritance.py b/tests/workflows/test_candidate_workflow_budget_inheritance.py index 0d6a328a..eee15c93 100644 --- a/tests/workflows/test_candidate_workflow_budget_inheritance.py +++ b/tests/workflows/test_candidate_workflow_budget_inheritance.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import json import subprocess import pytest @@ -14,6 +15,7 @@ from opencollab.adapters.llm.types import LLMResponse, Usage from opencollab.application.workflow import WorkflowBudgetExceeded, WorkflowContext from opencollab.bootstrap._workflow_runtime_session import WorkflowSessionFactory +from tests.support.provider_sdk_http import completion_http_response, install_sdk_transport from tests.support.workflow_context_test_support import FakeFactory, FakeSession @@ -382,3 +384,90 @@ async def nested(child, _args): assert parent.pending_cleanup_tasks == () await parent.agent("later work") assert factory.builds[-1]["budget"] == 50 + + +@pytest.mark.parametrize("spent", [30, 130]) +async def test_parent_sees_candidate_spend_before_candidate_completion(repository, spent): + factory = FakeFactory([FakeSession(tokens=spent), FakeSession(tokens=7)]) + parent = _context(repository, factory, 100) + ready, release = asyncio.Event(), asyncio.Event() + children = [] + + async def nested(child, _args): + children.append(child) + await child.agent("candidate role") + ready.set() + await release.wait() + return "done" + + task = asyncio.create_task(parent.candidate_workflow(nested, {}, label="candidate", budget=80)) + try: + await asyncio.wait_for(ready.wait(), 10) + assert parent.tokens_spent() == spent + assert len(parent.sessions) == 1 + assert parent.tokens_remaining() == 100 - spent - max(0, 80 - spent) + assert children[0].tokens_remaining() == 80 - spent + if spent > 100: + with pytest.raises(WorkflowBudgetExceeded): + await parent.agent("later parent role", budget=20) + assert len(factory.builds) == 1 + else: + assert await parent.agent("later parent role", budget=20) == "done" + assert children[0].tokens_remaining() == 50 + finally: + release.set() + await asyncio.wait_for(task, 10) + + expected = spent + (7 if spent < 100 else 0) + assert parent.tokens_spent() == expected + assert len(parent.sessions) == (2 if spent < 100 else 1) + assert len({id(session) for session in parent.sessions}) == len(parent.sessions) + assert parent.tokens_remaining() == 100 - expected + + +async def test_native_candidate_settlement_stops_parent_admission_while_child_is_open(repository, monkeypatch): + requests, contexts, observations = [], [], [] + ready, release = asyncio.Event(), asyncio.Event() + + async def handler(request): + body = json.loads(request.content) + requests.append(body) + response = completion_http_response(request, text="done") + if len(requests) == 1: + payload = json.loads(response.content) + payload["usage"] = {"prompt_tokens": 11793, "completion_tokens": 1, "total_tokens": 11794} + return type(response)(200, json=payload) + return response + + install_sdk_transport(monkeypatch, "openai", handler) + + async def candidate(child, _args): + await child.agent("Summarize the supplied diagnostics.", tools=[], budget=8000) + ready.set() + await release.wait() + return "done" + + async def outer(ctx, _args): + contexts.append(ctx) + task = asyncio.create_task(ctx.candidate_workflow(candidate, {}, label="candidate", budget=8000)) + try: + await asyncio.wait_for(ready.wait(), 10) + observations.append((ctx.tokens_spent(), ctx.tokens_remaining(), len(ctx.sessions))) + with pytest.raises(WorkflowBudgetExceeded): + await ctx.agent("Produce the final summary.", tools=[], budget=2000) + finally: + release.set() + await asyncio.wait_for(task, 10) + return "settled" + + result = await OpenCollab( + repository, model="gpt-4o", provider="openai", api_key="controlled-test", # pragma: allowlist secret + base_url="https://fixture.invalid/v1", config={"llm_max_retries": 0}, + ).workflow(outer, budget=10000, concurrency=1, max_steps=1, + limit_mode="explicit", trace=False) + + assert result.ok and result.output == "settled" + assert len(requests) == 1 + assert observations == [(11794, -1794, 1)] + assert result.tokens == contexts[0].tokens_spent() == 11794 + assert len(contexts[0].sessions) == 1 diff --git a/tests/workflows/test_weave_command_checks.py b/tests/workflows/test_weave_command_checks.py new file mode 100644 index 00000000..ef85f762 --- /dev/null +++ b/tests/workflows/test_weave_command_checks.py @@ -0,0 +1,271 @@ +"""Weave checks execute declared behavior and retain native test proof.""" + +from __future__ import annotations + +import shlex +import sys + +import pytest + +from opencollab import OpenCollab +from opencollab.builtin_workflows import WeaveAdapter +from tests.workflows.test_weave_execution import FileModel + + +async def _check(workspace, command, seconds=10): + async def flow(ctx, _args): + check = await WeaveAdapter(check_commands=[command]).verify(ctx, None, seconds) + return {"ok": check.ok, "executed": check.executed, "report": check.report} + + result = await OpenCollab(workspace, model="offline").workflow(flow, trace=False, timeout=20) + assert result.ok, result.reason + return result.output + + +@pytest.mark.parametrize("passed", [False, True]) +@pytest.mark.parametrize("wrapper", ["", "env CHECK_ENV=yes "]) +async def test_echo_prefix_executes_actual_behavior_probe(tmp_path, passed, wrapper): + (tmp_path / "check.py").write_text( + "from pathlib import Path\n" + "Path('executed').write_text('yes')\n" + f"assert {passed!r}\n" + "print('CHECK_OK')\n", + encoding="utf-8", + ) + command = f"echo 'Starting check' && {wrapper}{shlex.quote(sys.executable)} check.py" + output = await _check(tmp_path, {"command": command, "expected_output": "CHECK_OK"}) + assert (tmp_path / "executed").read_text() == "yes" + assert output["executed"] is True + assert output["ok"] is passed + + +@pytest.mark.parametrize("command", [ + "echo CHECK_OK", "printf CHECK_OK", "echo Starting && echo CHECK_OK", + "echo Starting && printf CHECK_OK", "echo 'Starting && python check.py CHECK_OK'", + "echo CHECK_OK # && python check.py", "echo CHECK_OK && echo '&&' python check.py", + "echo CHECK_OK | cat", "echo CHECK_OK; printf CHECK_OK", "echo Starting && /bin/echo CHECK_OK", + "echo Starting && env printf CHECK_OK", "echo Starting && command printf CHECK_OK", + "echo Starting && /usr/bin/env CHECK_ENV=yes printf CHECK_OK", + "echo Starting && (printf CHECK_OK)", "echo Starting && { printf CHECK_OK; }", + "echo Starting && true; printf CHECK_OK", "echo Starting && :; printf CHECK_OK", + "echo Starting && false || printf CHECK_OK", "echo Starting && false | printf CHECK_OK", + "echo Starting && false & printf CHECK_OK", + "echo Starting && true\nprintf CHECK_OK", "echo Starting && :\nprintf CHECK_OK", + "echo Starting && false\nprintf CHECK_OK", + "echo Starting && false # a comment\nprintf CHECK_OK", +]) +async def test_static_printing_compounds_remain_unverified(tmp_path, command): + output = await _check(tmp_path, {"command": command, "expected_output": "CHECK_OK"}) + assert output["executed"] is False + assert output["ok"] is False + + +@pytest.mark.parametrize("group", ["({command})", "{{ {command}; }}"]) +async def test_grouped_behavior_probes_remain_unverified(tmp_path, group): + (tmp_path / "check.py").write_text( + "from pathlib import Path\n" + "Path('executed').write_text('yes')\n" + "assert 6 * 7 == 42\n" + "print('CHECK_OK')\n", + encoding="utf-8", + ) + command = "echo Starting && " + group.format(command=f"{shlex.quote(sys.executable)} check.py") + output = await _check(tmp_path, {"command": command, "expected_output": "CHECK_OK"}) + assert output["executed"] is False and output["ok"] is False + assert not (tmp_path / "executed").exists() + + +@pytest.mark.parametrize("quote", ["'", '"']) +async def test_quoted_group_characters_preserve_real_probe_execution(tmp_path, quote): + target = "check (result) {value}.py" + (tmp_path / target).write_text( + "from pathlib import Path\nimport sys\n" + "assert sys.argv[1] == 'argument (result) {value} ; | & &&'\n" + "Path('executed').write_text('yes')\n" + "print('CHECK_OK')\n", + encoding="utf-8", + ) + command = f"echo {quote}Starting (check) {{value}}{quote} && {shlex.quote(sys.executable)} " + command += f"{quote}{target}{quote} {quote}argument (result) {{value}} ; | & &&{quote}" + output = await _check(tmp_path, {"command": command, "expected_output": "CHECK_OK"}) + assert output["executed"] is True and output["ok"] is True + assert (tmp_path / "executed").read_text() == "yes" + + +async def test_escaped_control_characters_preserve_real_probe_execution(tmp_path): + (tmp_path / "check.py").write_text( + "from pathlib import Path\nimport sys\n" + "assert sys.argv[1] == 'argument;|& '\n" + "Path('executed').write_text('yes')\n" + "print('CHECK_OK')\n", + encoding="utf-8", + ) + command = f"echo Starting && {shlex.quote(sys.executable)} check.py argument\\;\\|\\&\\ " + output = await _check(tmp_path, {"command": command, "expected_output": "CHECK_OK"}) + assert output["executed"] is True and output["ok"] is True + assert (tmp_path / "executed").read_text() == "yes" + + +@pytest.mark.parametrize("padding", [("", ""), ("", "\n"), ("", "\r\n"), (" \n\t", " \t\r\n")]) +async def test_probe_padding_preserves_real_probe_execution(tmp_path, padding): + (tmp_path / "check.py").write_text( + "from pathlib import Path\nimport sys\n" + "assert len(sys.argv) == 1\n" + "assert 6 * 7 == 42\n" + "Path('executed').write_text('yes')\n" + "print('CHECK_OK')\n", + encoding="utf-8", + ) + command = padding[0] + f"{shlex.quote(sys.executable)} check.py" + padding[1] + output = await _check(tmp_path, {"command": command, "expected_output": "CHECK_OK"}) + assert output["executed"] is True and output["ok"] is True + assert (tmp_path / "executed").read_text() == "yes" + assert output["report"]["checks"][0]["command"] == command + + +@pytest.mark.parametrize("argument", ["pytest", "unittest", "/opt/runners/pytest"]) +@pytest.mark.parametrize("prefix", ["", "echo Starting && env CHECK_ENV=yes "]) +async def test_script_runner_argument_preserves_assertion_probe_delivery(tmp_path, argument, prefix): + (tmp_path / "runner.txt").write_text(argument, encoding="utf-8") + (tmp_path / "check_runner.py").write_text( + "from pathlib import Path\nimport sys\n" + "assert Path('runner.txt').read_text() == sys.argv[1]\n" + "assert Path('result.txt').read_text() == 'correct\\n'\n" + "Path('executed').write_text('yes')\n" + "print('CHECK_OK')\n", + encoding="utf-8", + ) + command = f"{prefix}{shlex.quote(sys.executable)} check_runner.py {shlex.quote(argument)}" + result = await OpenCollab(tmp_path, model="offline").workflow( + "weave", {"groups": [{"id": "file", "prompt": "write-stage writes result.txt"}], + "check_commands": [{"command": command, "expected_output": "CHECK_OK"}], + "config": {"max_steps": 4, "max_repair_rounds": 0}}, + llm=FileModel(initial="correct\n"), budget=20_000, limit_mode="explicit", trace=False, timeout=20, + ) + assert result.ok, result.reason + assert (tmp_path / "executed").read_text() == "yes" + assert result.output["status"] == "completed" and result.output["delivery_ok"] is True + row = result.output["verification"]["report"]["checks"][0] + assert row["executed"] is True and row["exit_code"] == 0 and row["matched"] is True + assert not row["test_records"] + + +@pytest.mark.parametrize("target", ["test_result.py", "test result.py"]) +@pytest.mark.parametrize("separator", [False, True]) +@pytest.mark.parametrize("padding", [("", ""), ("", "\n"), ("", "\r\n"), (" \n\t", " \t\r\n")]) +async def test_documented_quiet_pytest_verifies_actual_target(tmp_path, target, separator, padding): + (tmp_path / target).write_text( + "from pathlib import Path\n" + "def test_result():\n" + " Path('test-executed').write_text('yes')\n" + " assert Path('result.txt').read_text() == 'correct\\n'\n", + encoding="utf-8", + ) + command = f"{shlex.quote(sys.executable)} -m pytest -q -p no:cacheprovider " + command += ("-- " if separator else "") + shlex.quote(target) + command = padding[0] + command + padding[1] + result = await OpenCollab(tmp_path, model="offline").workflow( + "weave", {"groups": [{"id": "file", "prompt": "write-stage writes result.txt"}], + "check_commands": [command], "config": {"max_steps": 4, "max_repair_rounds": 0}}, + llm=FileModel(initial="correct\n"), budget=20_000, limit_mode="explicit", trace=False, timeout=20, + ) + assert result.ok, result.reason + assert (tmp_path / "test-executed").read_text() == "yes" + assert result.output["delivery_ok"] is True + row = result.output["verification"]["report"]["checks"][0] + assert row["command"] == command + assert row["test_records"] and all(record["verified"] for record in row["test_records"]) + assert f"PASSED {target}::test_result" in row["output"] + + +async def test_quiet_pytest_preserves_cd_quotes_variables_and_selected_test(tmp_path, monkeypatch): + directory = tmp_path / "check directory" + directory.mkdir() + (directory / "test result.py").write_text( + "from pathlib import Path\n" + "def test_selected():\n" + " Path('selected').write_text('yes')\n" + " assert 6 * 7 == 42\n" + "def test_other():\n" + " raise AssertionError('selection was lost')\n", + encoding="utf-8", + ) + monkeypatch.setenv("OC_TEST_SELECTION", "test_selected") + command = f"cd -- 'check directory' && {shlex.quote(sys.executable)} -m pytest -q -p no:cacheprovider " + command += '-k "$OC_TEST_SELECTION" -- "test result.py"' + output = await _check(tmp_path, command) + assert output["ok"] is True + assert (directory / "selected").read_text() == "yes" + row = output["report"]["checks"][0] + assert row["executed_command"] == command.replace('-- "test result.py"', '-rA -- "test result.py"') + assert "1 passed, 1 deselected" in row["output"] + + +@pytest.mark.parametrize("mode", ["passed", "failed", "empty", "help", "collect", "compound", "redirect", "suffix"]) +@pytest.mark.parametrize("prefix", ["", "echo Starting && ", "echo Starting && env CHECK_ENV=yes "]) +async def test_pytest_adverse_results_and_unsupported_shell_remain_unverified(tmp_path, mode, prefix): + (tmp_path / "test_result.py").write_text( + "from pathlib import Path\n" + "def test_result():\n" + " Path('test-executed').write_text('yes')\n" + f" assert {mode != 'failed'!r}\n", + encoding="utf-8", + ) + (tmp_path / "empty").mkdir() + command = f"{prefix}{shlex.quote(sys.executable)} -m pytest -q -p no:cacheprovider " + command += {"empty": "empty", "help": "--help", "collect": "test_result.py --collect-only"}.get( + mode, "test_result.py") + command += {"compound": " && echo CHECK_OK", "redirect": " > result.log", "suffix": "; echo CHECK_OK"}.get(mode, "") + expected = {"passed": "passed", "empty": "no tests ran", "help": "usage", "collect": "collected", "redirect": "", + "compound": "CHECK_OK", "suffix": "CHECK_OK"}.get(mode, "failed") + output = await _check(tmp_path, {"command": command, "expected_output": expected or "CHECK_OK"}) + assert output["ok"] is (mode == "passed" and not prefix) + row = output["report"]["checks"][0] + if mode in {"help", "collect", "suffix"}: + assert output["executed"] is False + assert not (tmp_path / "test-executed").exists() + else: + assert output["executed"] is True + if mode != "empty": + assert (tmp_path / "test-executed").read_text() == "yes" + if prefix or mode in {"compound", "redirect", "suffix"}: + assert not row["test_records"] + assert "executed_command" not in row + assert "requires per-target test evidence" in row["reason"] + + +async def test_timed_out_behavior_probe_retains_execution_without_passing(tmp_path): + (tmp_path / "check.py").write_text( + "from pathlib import Path\nimport time\n" + "Path('started').write_text('yes')\nprint('CHECK_OK', flush=True)\n" + "time.sleep(5)\nPath('late').write_text('yes')\n", + encoding="utf-8", + ) + command = f"echo Starting && {shlex.quote(sys.executable)} check.py" + output = await _check(tmp_path, {"command": command, "expected_output": "CHECK_OK"}, seconds=.2) + assert (tmp_path / "started").read_text() == "yes" + assert not (tmp_path / "late").exists() + assert output["executed"] is True and output["ok"] is False + assert output["report"]["checks"][0]["exit_code"] != 0 + + +async def test_truncated_pytest_output_retains_failed_evidence(tmp_path, monkeypatch): + from opencollab.adapters import _env_local + + monkeypatch.setattr(_env_local, "PROCESS_OUTPUT_CAPTURE_BYTES", 512) + (tmp_path / "test_result.py").write_text( + "from pathlib import Path\n" + "def test_result():\n" + " Path('test-executed').write_text('yes')\n" + " print('x' * 5000)\n" + " assert 6 * 7 == 42\n", + encoding="utf-8", + ) + command = f"{shlex.quote(sys.executable)} -m pytest test_result.py -q -s -p no:cacheprovider" + output = await _check(tmp_path, command) + assert (tmp_path / "test-executed").read_text() == "yes" + row = output["report"]["checks"][0] + assert row["exit_code"] == 0 + assert "1 passed" in row["output"] + assert row["test_records"] and all(not record["verified"] for record in row["test_records"]) + assert output["executed"] is True and output["ok"] is False diff --git a/tests/workflows/test_workflow_context_parallel.py b/tests/workflows/test_workflow_context_parallel.py index 2afa2215..0afd213b 100644 --- a/tests/workflows/test_workflow_context_parallel.py +++ b/tests/workflows/test_workflow_context_parallel.py @@ -10,6 +10,7 @@ from opencollab.application.workflow import ( WorkflowBudgetExceeded, WorkflowContext, + WorkflowEnvironmentRevoked, ) from tests.support.workflow_context_test_support import ( FakeFactory, @@ -682,3 +683,57 @@ async def later_stage(prev: Any, item: int, idx: int) -> Any: with pytest.raises(WorkflowBudgetExceeded): await ctx.pipeline([7], agent_stage, later_stage) assert later_ran == [] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("kind", ["parallel", "pipeline"]) +@pytest.mark.parametrize("error_type", [WorkflowBudgetExceeded, WorkflowEnvironmentRevoked]) +async def test_collections_preserve_returned_terminal_error_values(kind, error_type): + ctx = WorkflowContext(FakeFactory([])) + diagnostic = error_type("earlier operation failed") + if kind == "parallel": + values = await ctx.parallel([lambda: asyncio.sleep(0, result=diagnostic)]) + else: + values = await ctx.pipeline([diagnostic]) + assert values[0] is diagnostic + + +@pytest.mark.asyncio +@pytest.mark.parametrize("kind", ["parallel", "pipeline"]) +@pytest.mark.parametrize("error_types", [ + (WorkflowEnvironmentRevoked, WorkflowEnvironmentRevoked), + (WorkflowEnvironmentRevoked, WorkflowBudgetExceeded), + (WorkflowBudgetExceeded, WorkflowEnvironmentRevoked), + (WorkflowBudgetExceeded, WorkflowBudgetExceeded), +]) +async def test_collections_choose_thrown_errors_by_budget_priority_then_input_order(kind, error_types): + ctx = WorkflowContext(FakeFactory([]), task_concurrency=2) + errors = [error_type(f"unit {index}") for index, error_type in enumerate(error_types)] + first_started, second_raised = asyncio.Event(), asyncio.Event() + raised_order = [] + + async def unit(index): + if index == 0: + first_started.set() + await second_raised.wait() + elif index == 1: + await first_started.wait() + second_raised.set() + else: + pytest.fail("queued work started after a terminal error") + raised_order.append(index) + raise errors[index] + + if kind == "parallel": + operation = ctx.parallel([lambda index=index: unit(index) for index in range(3)]) + else: + async def stage(_previous, _item, index): + return await unit(index) + + operation = ctx.pipeline(list(range(3)), stage) + expected = next((error for error in errors if isinstance(error, WorkflowBudgetExceeded)), errors[0]) + with pytest.raises(type(expected)) as caught: + await asyncio.wait_for(operation, timeout=1) + assert caught.value is expected + assert raised_order == [1, 0] + assert ctx._task_semaphore._value == 2 diff --git a/tests/workflows/test_workflow_runtime_cleanup.py b/tests/workflows/test_workflow_runtime_cleanup.py index 09ddca17..93a08adf 100644 --- a/tests/workflows/test_workflow_runtime_cleanup.py +++ b/tests/workflows/test_workflow_runtime_cleanup.py @@ -147,12 +147,14 @@ class CancellationResistantSession: def __init__(self, environment, tracer) -> None: self.env = environment self.tracer = tracer + self.started = asyncio.Event() self.cancel_seen = asyncio.Event() async def add_user_message(self, content: str) -> None: return None async def run_loop(self) -> str: + self.started.set() try: await asyncio.Event().wait() except asyncio.CancelledError: @@ -204,7 +206,11 @@ def recording_manifest(*args, **kwargs): ) async def fn(ctx, args): - assert await ctx.agent("slow", timeout=0.001) is None + agent_task = asyncio.create_task(ctx.agent("slow")) + await asyncio.wait_for(holder["session"].started.wait(), timeout=MUST_SETTLE_SECONDS) + agent_task.cancel() + with pytest.raises(asyncio.CancelledError): + await asyncio.wait_for(agent_task, timeout=MUST_SETTLE_SECONDS) await asyncio.wait_for(holder["session"].cancel_seen.wait(), timeout=MUST_SETTLE_SECONDS) return "ok" @@ -614,6 +620,7 @@ class PermanentlyCancellationResistantSession: def __init__(self, environment, tracer) -> None: self.env = environment self.tracer = tracer + self.started = asyncio.Event() self.cancel_seen = asyncio.Event() self.release = asyncio.Event() @@ -621,6 +628,7 @@ async def add_user_message(self, content: str) -> None: return None async def run_loop(self) -> str: + self.started.set() try: await asyncio.Event().wait() except asyncio.CancelledError: @@ -661,7 +669,11 @@ def fake_build_context(**kwargs): monkeypatch.setattr(workflow_execution, "build_workflow_context", fake_build_context) async def fn(ctx, args): - assert await ctx.agent("slow", timeout=0.001) is None + agent_task = asyncio.create_task(ctx.agent("slow")) + await asyncio.wait_for(holder["session"].started.wait(), timeout=MUST_SETTLE_SECONDS) + agent_task.cancel() + with pytest.raises(asyncio.CancelledError): + await asyncio.wait_for(agent_task, timeout=MUST_SETTLE_SECONDS) await asyncio.wait_for(holder["session"].cancel_seen.wait(), timeout=MUST_SETTLE_SECONDS) return "workflow-returned"