From 22abc1bbc61bc543116bdbd62a1db7213544213d Mon Sep 17 00:00:00 2001 From: shuofang <916931057@qq.com> Date: Tue, 16 Jun 2026 18:53:55 +0800 Subject: [PATCH 1/2] Contract model-visible terminal tools --- Dockerfile | 2 +- open_terminal/main.py | 425 ++++++++++++++++++++++++++++- open_terminal/utils/apply_patch.py | 266 ++++++++++++++++++ tests/test_tool_contract.py | 271 ++++++++++++++++++ 4 files changed, 949 insertions(+), 15 deletions(-) create mode 100644 open_terminal/utils/apply_patch.py create mode 100644 tests/test_tool_contract.py diff --git a/Dockerfile b/Dockerfile index c0a3acb..ea90c71 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,7 +4,7 @@ FROM python:3.12.13 RUN apt-get update && apt-get install -y --no-install-recommends \ # Core utilities - coreutils findutils grep sed gawk diffutils patch \ + coreutils findutils grep ripgrep sed gawk diffutils patch \ less file tree bc man-db \ # Networking curl wget net-tools iputils-ping dnsutils netcat-openbsd socat telnet \ diff --git a/open_terminal/main.py b/open_terminal/main.py index c035741..6bb2bed 100644 --- a/open_terminal/main.py +++ b/open_terminal/main.py @@ -3,6 +3,7 @@ from importlib.metadata import version as _pkg_version import fnmatch import json +import subprocess import aiofiles import aiofiles.os @@ -18,13 +19,19 @@ from dataclasses import dataclass, field from typing import Optional -from fastapi import Depends, FastAPI, File, HTTPException, Query, Request, UploadFile, WebSocket, WebSocketDisconnect +from fastapi import Depends, FastAPI, File, HTTPException, Path as PathParam, Query, Request, UploadFile, WebSocket, WebSocketDisconnect from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse, Response from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer from pydantic import BaseModel, Field from open_terminal.env import API_KEY, BINARY_FILE_MIME_PREFIXES, CORS_ALLOWED_ORIGINS, ENABLE_NOTEBOOKS, ENABLE_SYSTEM_PROMPT, ENABLE_TERMINAL, EXECUTE_DESCRIPTION, EXECUTE_TIMEOUT, LOG_DIR, MAX_TERMINAL_SESSIONS, MULTI_USER, OPEN_TERMINAL_INFO, PROCESS_LOG_RETENTION, SESSION_CWD_TTL, SYSTEM_PROMPT, TERMINAL_TERM +from open_terminal.utils.apply_patch import ( + PatchParseError, + commit_staged_patch, + parse_apply_patch_text, + stage_apply_patch, +) from open_terminal.utils.runner import PipeRunner, ProcessRunner, create_runner from open_terminal.utils.fs import UserFS @@ -44,7 +51,6 @@ import fcntl import pty import struct - import subprocess import termios _PTY_AVAILABLE = True @@ -63,6 +69,62 @@ def get_system_info() -> str: ) +_CLI_CONTRACT_COMMANDS = ( + "rg", + "git", + "jq", + "python3", + "node", + "curl", + "tar", + "zip", + "unzip", + "find", + "sed", + "awk", + "file", + "patch", + "diff", +) + + +def _probe_cli_version(command: str) -> dict: + executable = shutil.which(command) + if not executable: + return {"available": False, "path": None, "version": None} + + version_args = [executable, "--version"] + if command == "node": + version_args = [executable, "--version"] + + try: + completed = subprocess.run( + version_args, + check=False, + capture_output=True, + text=True, + timeout=2, + ) + except Exception: + if command == "awk": + try: + completed = subprocess.run( + [executable, "-W", "version"], + check=False, + capture_output=True, + text=True, + timeout=2, + ) + except Exception: + return {"available": True, "path": executable, "version": None} + else: + return {"available": True, "path": executable, "version": None} + + output = (completed.stdout or completed.stderr or "").strip() + first_line = output.splitlines()[0] if output else None + return {"available": True, "path": executable, "version": first_line} + + def get_system_prompt() -> str: """Build a default system prompt for LLM integration.""" if SYSTEM_PROMPT: @@ -89,7 +151,13 @@ def get_system_prompt() -> str: _EXECUTE_DESCRIPTION = ( - "Run a shell command in the background and return a command ID.\n\n" + "Run a shell command in the background and return a process_id. " + "Use this as the primary system tool for filesystem search, git, package, " + "build, test, and diagnostic commands. Relative paths resolve against the " + "session cwd, or the supplied cwd when provided. If wait is omitted or the " + "command is still running, poll get_process_status with the returned " + "process_id to read output and completion status. Use send_process_input " + "for interactive stdin and kill_process to terminate long-running commands.\n\n" + get_system_info() ) if EXECUTE_DESCRIPTION: @@ -191,6 +259,31 @@ class WriteRequest(BaseModel): ..., description="Text content to write to the file.", ) + overwrite: bool = Field( + False, + description="Defaults to false. If false, writing to an existing path returns a 409 conflict instead of replacing it.", + ) + + +class ApplyPatchRequest(BaseModel): + patch: str = Field( + ..., + description=( + "Patch text using the OpenAI apply_patch format. Must start with " + "'*** Begin Patch', contain one or more hunks such as " + "'*** Add File:', '*** Update File:', or '*** Delete File:', and end " + "with '*** End Patch'." + ), + json_schema_extra={ + "examples": [ + "*** Begin Patch\n*** Update File: path/to/file.py\n@@\n-old line\n+new line\n*** End Patch" + ] + }, + ) + dry_run: bool = Field( + False, + description="If true, validate and report changes without writing to disk.", + ) class ReplacementChunk(BaseModel): @@ -247,6 +340,111 @@ class ReplaceRequest(BaseModel): ) +class CliVersionInfo(BaseModel): + available: bool = Field(..., description="Whether the executable was found on PATH.") + path: Optional[str] = Field(None, description="Resolved executable path, or null when unavailable.") + version: Optional[str] = Field(None, description="First line of version output, or null when unavailable.") + + +class EnvironmentOSInfo(BaseModel): + system: str = Field(..., description="Operating system name, for example Linux, Darwin, or Windows.") + release: str = Field(..., description="Operating system release.") + version: str = Field(..., description="Operating system version string.") + machine: str = Field(..., description="Machine architecture.") + python: str = Field(..., description="Python runtime version used by Open Terminal.") + + +class EnvironmentPermissionsInfo(BaseModel): + multi_user: bool = Field(..., description="Whether Open Terminal is running in multi-user isolation mode.") + run_as_user: Optional[str] = Field(None, description="Provisioned OS user used for commands and file operations, if any.") + api_key_required: bool = Field(..., description="Whether API key authentication is enabled.") + path_boundary: str = Field(..., description="Effective file access boundary, such as own_home_only or server_process.") + + +class EnvironmentResponse(BaseModel): + os: EnvironmentOSInfo = Field(..., description="Operating system and Python runtime metadata.") + hostname: str = Field(..., description="Host name reported by the runtime.") + user: str = Field(..., description="Effective user for this request.") + home: str = Field(..., description="Default home directory for this request.") + cwd: str = Field(..., description="Current session working directory.") + shell: str = Field(..., description="Default shell path.") + environment: dict[str, str] = Field(..., description="Selected environment variables such as PATH.") + cli_versions: dict[str, CliVersionInfo] = Field(..., description="Availability and version probe for the stable CLI contract.") + permissions: EnvironmentPermissionsInfo = Field(..., description="Authentication and path-boundary metadata.") + info: Optional[str] = Field(None, description="Operator-provided environment info, if configured.") + + +class ReadFileResponse(BaseModel): + path: str = Field(..., description="Resolved file path that was read.") + total_lines: int = Field(..., description="Total number of lines in the text or extracted document.") + content: str = Field(..., description="Returned text content for the requested line range.") + + +class DisplayFileResponse(BaseModel): + path: str = Field(..., description="Resolved path that the client should display to the user.") + exists: bool = Field(..., description="Whether the resolved path currently exists as a file.") + + +class WriteFileResponse(BaseModel): + path: str = Field(..., description="Resolved file path that was written.") + size: int = Field(..., description="Number of UTF-8 bytes written.") + + +class PatchConflictInfo(BaseModel): + path: str = Field(..., description="Resolved path where the conflict occurred.") + reason: str = Field(..., description="Reason the patch could not be applied.") + + +class ErrorResponse(BaseModel): + detail: str = Field(..., description="Human-readable error detail.") + + +class ApplyPatchConflictDetail(BaseModel): + message: str = Field(..., description="Conflict summary.") + conflicts: list[PatchConflictInfo] = Field(..., description="Patch conflicts that prevented applying any changes.") + + +class ApplyPatchConflictResponse(BaseModel): + detail: ApplyPatchConflictDetail = Field(..., description="Structured patch conflict detail.") + + +class PatchChangeInfo(BaseModel): + type: str = Field(..., description="Change type: add, update, delete, or move/update.") + path: str = Field(..., description="Resolved source path affected by the change.") + move_path: Optional[str] = Field(None, description="Resolved destination path for move hunks.") + size: Optional[int] = Field(None, description="UTF-8 byte size of the resulting file content, when applicable.") + + +class ApplyPatchResponse(BaseModel): + applied: bool = Field(..., description="True when changes were written to disk; false for dry_run.") + dry_run: bool = Field(..., description="Whether this request validated without writing.") + changes: list[PatchChangeInfo] = Field(..., description="Staged or applied changes.") + conflicts: list[PatchConflictInfo] = Field(..., description="Conflicts; empty for successful 200 responses.") + + +class ProcessSummaryResponse(BaseModel): + id: str = Field(..., description="process_id used with get_process_status, send_process_input, and kill_process.") + command: str = Field(..., description="Command string that was started.") + status: str = Field(..., description="Process status: running, done, or killed.") + exit_code: Optional[int] = Field(None, description="Process exit code when available.") + log_path: Optional[str] = Field(None, description="JSONL log path for persisted command output.") + + +class ProcessOutputEntry(BaseModel): + type: str = Field(..., description="Output stream type: stdout, stderr, or output for PTY-combined output.") + data: str = Field(..., description="Output text chunk.") + + +class ProcessStatusResponse(ProcessSummaryResponse): + output: list[ProcessOutputEntry] = Field(..., description="Output entries returned by this poll.") + truncated: bool = Field(..., description="Whether returned output was truncated by tail/log limits.") + next_offset: int = Field(..., description="Offset to pass to get_process_status to read only new output next time.") + + +class StatusResponse(BaseModel): + status: str = Field(..., description="Operation status.") + + # --------------------------------------------------------------------------- # Background process management @@ -378,6 +576,60 @@ async def get_config(): } +@app.get( + "/environment", + operation_id="get_environment", + summary="Get runtime environment", + description=( + "Return stable runtime metadata: OS, hostname, user, home, cwd, shell, " + "PATH, key CLI versions, and permission boundaries. Returns JSON fields: " + "os, hostname, user, home, cwd, shell, environment, cli_versions, " + "permissions, and info." + ), + response_model=EnvironmentResponse, + dependencies=[Depends(verify_api_key)], + responses={ + 401: {"description": "Invalid or missing API key."}, + }, +) +async def get_environment( + http_request: Request, + fs: UserFS = Depends(get_filesystem), +): + session_id = http_request.headers.get("x-session-id") + cwd = _get_session_cwd(session_id, fs) if session_id else fs.home + user = fs.username or os.environ.get("USER") or os.environ.get("USERNAME") or "unknown" + + return { + "os": { + "system": platform.system(), + "release": platform.release(), + "version": platform.version(), + "machine": platform.machine(), + "python": sys.version.split()[0], + }, + "hostname": socket.gethostname(), + "user": user, + "home": fs.home, + "cwd": cwd, + "shell": os.environ.get("SHELL", "/bin/sh"), + "environment": { + "PATH": os.environ.get("PATH", ""), + }, + "cli_versions": { + command: _probe_cli_version(command) + for command in _CLI_CONTRACT_COMMANDS + }, + "permissions": { + "multi_user": MULTI_USER, + "run_as_user": fs.username, + "api_key_required": bool(API_KEY), + "path_boundary": "own_home_only" if fs.username else "server_process", + }, + "info": OPEN_TERMINAL_INFO or None, + } + + if ENABLE_SYSTEM_PROMPT: @app.get( @@ -394,6 +646,7 @@ async def get_system(): @app.get( "/info", + include_in_schema=False, operation_id="get_info", summary="Get environment info", description="Return operator-provided information about this environment. Use this to understand the system you are working with.", @@ -441,6 +694,7 @@ async def set_cwd( @app.get( "/files/list", + include_in_schema=False, operation_id="list_files", summary="List directory contents", description="Return a structured listing of files and directories at the given path.", @@ -468,9 +722,24 @@ async def list_files( "/files/read", operation_id="read_file", summary="Read a file", - description="Read a file and return its contents. Supports text files and images (PNG, JPEG, WebP, etc.). For text files you can optionally request a specific line range. Images are returned as binary so you can view and analyze them directly. Use display_file to show a file to the user.", + description=( + "Read a file and return its contents. Text files and extracted documents " + "return JSON with path, total_lines, and content. For text files you can " + "request a line range. Supported images return raw HTTP binary data with " + "the image MIME type, not base64. Use display_file to show a file to the user." + ), + response_model=ReadFileResponse, dependencies=[Depends(verify_api_key)], responses={ + 200: { + "description": "Returns JSON for text/document content. Raw binary image data is returned for supported image MIME types.", + "content": { + "image/png": {"schema": {"type": "string", "format": "binary"}}, + "image/jpeg": {"schema": {"type": "string", "format": "binary"}}, + "image/webp": {"schema": {"type": "string", "format": "binary"}}, + "image/gif": {"schema": {"type": "string", "format": "binary"}}, + }, + }, 404: {"description": "File not found."}, 415: {"description": "Unsupported binary file type."}, 401: {"description": "Invalid or missing API key."}, @@ -543,7 +812,13 @@ async def read_file( "/files/display", operation_id="display_file", summary="Display a file to the user", - description="Open a file in the user's file viewer so they can see it. Use this when the user wants to view or look at a file. This does not return file content to you — use read_file if you need to read the content yourself.", + description=( + "Make a file visible to the user in the client preview/viewer. Use this " + "when the user wants to view or preview a file. This does not return file " + "content to you; use read_file if you need to read the content yourself. " + "Returns JSON with path and exists." + ), + response_model=DisplayFileResponse, dependencies=[Depends(verify_api_key)], responses={ 401: {"description": "Invalid or missing API key."}, @@ -608,9 +883,20 @@ async def serve_file(path: str, fs: UserFS = Depends(get_filesystem)): "/files/write", operation_id="write_file", summary="Write a file", - description="Write text content to a file. Creates parent directories automatically. Overwrites if the file already exists.", + description=( + "Write complete text content to a file. Parent directories are created " + "automatically. By default this tool does not overwrite existing files: " + "an existing path returns HTTP 409. Set overwrite=true only when replacing " + "the whole file is intended. Prefer apply_patch for localized edits. " + "Returns JSON with path and size." + ), + response_model=WriteFileResponse, dependencies=[Depends(verify_api_key)], responses={ + 409: { + "model": ErrorResponse, + "description": "File already exists and overwrite=false.", + }, 401: {"description": "Invalid or missing API key."}, }, ) @@ -618,6 +904,11 @@ async def write_file(http_request: Request, request: WriteRequest, fs: UserFS = session_id = http_request.headers.get("x-session-id") session_cwd = _get_session_cwd(session_id, fs) if session_id else None target = fs.resolve_path(request.path, cwd=session_cwd) + if not request.overwrite and await fs.exists(target): + raise HTTPException( + status_code=409, + detail="File already exists; set overwrite=true to replace it", + ) try: await fs.write(target, request.content) except (OSError, subprocess.CalledProcessError) as e: @@ -687,6 +978,7 @@ async def move_entry(request: MoveRequest, fs: UserFS = Depends(get_filesystem)) @app.post( "/files/replace", + include_in_schema=False, operation_id="replace_file_content", summary="Replace content in a file", description="Find and replace exact strings in a file. Supports multiple replacements in one call with optional line range narrowing.", @@ -745,8 +1037,81 @@ async def replace_file_content(http_request: Request, request: ReplaceRequest, f return {"path": target, "size": len(content.encode())} +@app.post( + "/files/apply_patch", + operation_id="apply_patch", + summary="Apply a patch", + description=( + "Apply an OpenAI apply_patch-format patch. The patch must use markers like " + "'*** Begin Patch', '*** Add File:', '*** Update File:', '*** Delete File:', " + "'*** Move to:', and '*** End Patch'. Supports multi-hunk updates, add, " + "delete, move, dry_run=true validation without writing, and structured " + "409 conflict reports when old content does not match. Returns JSON with " + "applied, dry_run, changes, and conflicts." + ), + response_model=ApplyPatchResponse, + dependencies=[Depends(verify_api_key)], + responses={ + 400: {"description": "Patch syntax is invalid."}, + 409: { + "model": ApplyPatchConflictResponse, + "description": "Patch could not be applied cleanly.", + }, + 401: {"description": "Invalid or missing API key."}, + }, +) +async def apply_patch( + http_request: Request, + request: ApplyPatchRequest, + fs: UserFS = Depends(get_filesystem), +): + session_id = http_request.headers.get("x-session-id") + session_cwd = _get_session_cwd(session_id, fs) if session_id else None + + try: + changes = parse_apply_patch_text(request.patch) + except PatchParseError as e: + raise HTTPException(status_code=400, detail=str(e)) + + staged, conflicts = await stage_apply_patch(changes, fs, session_cwd) + if conflicts: + raise HTTPException( + status_code=409, + detail={ + "message": "Patch could not be applied", + "conflicts": conflicts, + }, + ) + + if not request.dry_run: + try: + await commit_staged_patch(staged, fs) + except (OSError, subprocess.CalledProcessError) as e: + raise HTTPException(status_code=400, detail=str(e)) + + return { + "applied": not request.dry_run, + "dry_run": request.dry_run, + "changes": [ + { + "type": change.type, + "path": change.path, + "move_path": change.move_path, + "size": ( + len((change.new_content or "").encode()) + if change.new_content is not None + else None + ), + } + for change in staged + ], + "conflicts": [], + } + + @app.get( "/files/grep", + include_in_schema=False, operation_id="grep_search", summary="Search file contents", description="Search for a text pattern across files in a directory. Returns structured matches with file paths, line numbers, and matching lines. Skips binary files.", @@ -861,6 +1226,7 @@ def _search_file(file_path: str): @app.get( "/files/glob", + include_in_schema=False, operation_id="glob_search", summary="Search files by name", description="Search for files and subdirectories by name within a specified directory using glob patterns. Results will include the relative path, type, size, and modification time.", @@ -1075,7 +1441,11 @@ def _build_zip() -> bytes: "/execute", operation_id="list_processes", summary="List running commands", - description="Returns a list of all tracked background processes, including running, done, and killed.", + description=( + "Returns a JSON list of all tracked background processes, including " + "id, command, status, exit_code, and log_path." + ), + response_model=list[ProcessSummaryResponse], dependencies=[Depends(verify_api_key)], responses={ 401: {"description": "Invalid or missing API key."}, @@ -1100,6 +1470,7 @@ async def list_processes(): operation_id="run_command", summary="Execute a command", description=_EXECUTE_DESCRIPTION, + response_model=ProcessStatusResponse, dependencies=[Depends(verify_api_key)], responses={ 401: {"description": "Invalid or missing API key."}, @@ -1168,7 +1539,12 @@ async def execute( "/execute/{process_id}/status", operation_id="get_process_status", summary="Get command status and output", - description="Returns new output since the last poll, process status, and exit code. Output is drained on read to keep memory bounded.", + description=( + "Poll a process started by run_command. Returns output, status, exit_code, " + "next_offset, truncation state, and log_path. Use offset=next_offset from " + "the previous response to read only new output." + ), + response_model=ProcessStatusResponse, dependencies=[Depends(verify_api_key)], responses={ 404: {"description": "Process not found."}, @@ -1176,7 +1552,10 @@ async def execute( }, ) async def get_status( - process_id: str, + process_id: str = PathParam( + ..., + description="The process_id returned by run_command.", + ), wait: Optional[float] = Query( None, description="Seconds to wait for the process to finish before returning. Returns early if the process exits. Null to return immediately.", @@ -1226,7 +1605,12 @@ async def get_status( "/execute/{process_id}/input", operation_id="send_process_input", summary="Send input to a running command", - description="Write text to the process's stdin. Include newline characters as needed.", + description=( + "Write text to stdin for a running process started by run_command. Include " + "newlines when the command expects Enter. Literal escape sequences such as " + "\\n, \\x03, and \\x04 are converted before sending." + ), + response_model=StatusResponse, dependencies=[Depends(verify_api_key)], responses={ 404: {"description": "Process not found."}, @@ -1234,7 +1618,13 @@ async def get_status( 401: {"description": "Invalid or missing API key."}, }, ) -async def send_input(process_id: str, body: InputRequest): +async def send_input( + body: InputRequest, + process_id: str = PathParam( + ..., + description="The process_id returned by run_command.", + ), +): background_process = _get_process(process_id) if background_process.status != "running": raise HTTPException(status_code=400, detail="Process has already exited") @@ -1257,7 +1647,12 @@ async def send_input(process_id: str, body: InputRequest): "/execute/{process_id}", operation_id="kill_process", summary="Kill a running command", - description="Terminate the process. Sends SIGTERM by default for graceful shutdown. Use force=true to send SIGKILL.", + description=( + "Terminate the process. Sends SIGTERM by default for graceful shutdown " + "on Unix-like backends; use force=true to request SIGKILL or the closest " + "available forceful termination behavior." + ), + response_model=StatusResponse, dependencies=[Depends(verify_api_key)], responses={ 404: {"description": "Process not found."}, @@ -1265,7 +1660,10 @@ async def send_input(process_id: str, body: InputRequest): }, ) async def kill_process( - process_id: str, + process_id: str = PathParam( + ..., + description="The process_id returned by run_command.", + ), force: bool = Query(False, description="Send SIGKILL instead of SIGTERM."), ): background_process = _get_process(process_id) @@ -1783,4 +2181,3 @@ async def _pty_reader(): from open_terminal.utils.notebooks import create_notebooks_router app.include_router(create_notebooks_router(verify_api_key)) - diff --git a/open_terminal/utils/apply_patch.py b/open_terminal/utils/apply_patch.py new file mode 100644 index 0000000..671073a --- /dev/null +++ b/open_terminal/utils/apply_patch.py @@ -0,0 +1,266 @@ +from dataclasses import dataclass, field +from typing import Optional + +from open_terminal.utils.fs import UserFS + + +@dataclass +class PatchChunk: + context: Optional[str] + old_lines: list[str] + new_lines: list[str] + + +@dataclass +class ParsedPatchChange: + type: str + path: str + content: Optional[str] = None + chunks: list[PatchChunk] = field(default_factory=list) + move_path: Optional[str] = None + + +@dataclass +class StagedPatchChange: + type: str + path: str + new_content: Optional[str] = None + move_path: Optional[str] = None + + +class PatchParseError(ValueError): + pass + + +def _is_patch_hunk_marker(line: str) -> bool: + return ( + line.startswith("*** Add File: ") + or line.startswith("*** Delete File: ") + or line.startswith("*** Update File: ") + ) + + +def _patch_lines_to_text(lines: list[str]) -> str: + if not lines: + return "" + return "\n".join(lines) + "\n" + + +def parse_apply_patch_text(patch: str) -> list[ParsedPatchChange]: + lines = patch.splitlines() + while lines and not lines[0].strip(): + lines.pop(0) + while lines and not lines[-1].strip(): + lines.pop() + + if not lines or lines[0].strip() != "*** Begin Patch": + raise PatchParseError("The first line of the patch must be '*** Begin Patch'") + if len(lines) < 2 or lines[-1].strip() != "*** End Patch": + raise PatchParseError("The last line of the patch must be '*** End Patch'") + + changes: list[ParsedPatchChange] = [] + i = 1 + end = len(lines) - 1 + while i < end: + line = lines[i] + if line.startswith("*** Add File: "): + path = line[len("*** Add File: "):].strip() + i += 1 + content_lines: list[str] = [] + while i < end and not _is_patch_hunk_marker(lines[i]): + if not lines[i].startswith("+"): + raise PatchParseError(f"Invalid add-file line at patch line {i + 1}") + content_lines.append(lines[i][1:]) + i += 1 + changes.append( + ParsedPatchChange( + type="add", + path=path, + content=_patch_lines_to_text(content_lines), + ) + ) + continue + + if line.startswith("*** Delete File: "): + path = line[len("*** Delete File: "):].strip() + changes.append(ParsedPatchChange(type="delete", path=path)) + i += 1 + continue + + if line.startswith("*** Update File: "): + path = line[len("*** Update File: "):].strip() + i += 1 + move_path = None + if i < end and lines[i].startswith("*** Move to: "): + move_path = lines[i][len("*** Move to: "):].strip() + i += 1 + + chunks: list[PatchChunk] = [] + current: Optional[PatchChunk] = None + + def flush_current(): + nonlocal current + if current and (current.old_lines or current.new_lines): + chunks.append(current) + current = None + + while i < end and not _is_patch_hunk_marker(lines[i]): + patch_line = lines[i] + if patch_line == "*** End of File": + i += 1 + continue + if patch_line.startswith("@@"): + flush_current() + context = patch_line[3:] if patch_line.startswith("@@ ") else None + current = PatchChunk(context=context, old_lines=[], new_lines=[]) + i += 1 + continue + + if not patch_line: + raise PatchParseError(f"Invalid empty patch line at patch line {i + 1}") + prefix = patch_line[0] + value = patch_line[1:] + if prefix not in (" ", "+", "-"): + raise PatchParseError(f"Invalid update line at patch line {i + 1}") + if current is None: + current = PatchChunk(context=None, old_lines=[], new_lines=[]) + if prefix == " ": + current.old_lines.append(value) + current.new_lines.append(value) + elif prefix == "-": + current.old_lines.append(value) + else: + current.new_lines.append(value) + i += 1 + + flush_current() + if not chunks and move_path is None: + raise PatchParseError(f"Update file has no changes: {path}") + changes.append( + ParsedPatchChange( + type="update", + path=path, + chunks=chunks, + move_path=move_path, + ) + ) + continue + + raise PatchParseError(f"Invalid patch hunk at patch line {i + 1}") + + if not changes: + raise PatchParseError("Patch must contain at least one hunk") + return changes + + +def _replace_chunk_once(content: str, chunk: PatchChunk, start_at: int) -> tuple[str, int] | None: + old_text = _patch_lines_to_text(chunk.old_lines) + new_text = _patch_lines_to_text(chunk.new_lines) + search_start = start_at + + if chunk.context: + context_text = chunk.context + context_index = content.find(context_text, search_start) + if context_index == -1: + context_index = content.find(context_text + "\n", search_start) + if context_index == -1: + return None + search_start = context_index + len(context_text) + + old_index = content.find(old_text, search_start) + matched_old = old_text + if old_index == -1 and old_text.endswith("\n"): + matched_old = old_text[:-1] + old_index = content.find(matched_old, search_start) + if old_index == -1: + return None + + updated = content[:old_index] + new_text + content[old_index + len(matched_old):] + return updated, old_index + len(new_text) + + +async def stage_apply_patch( + changes: list[ParsedPatchChange], + fs: UserFS, + cwd: Optional[str], +) -> tuple[list[StagedPatchChange], list[dict]]: + staged: list[StagedPatchChange] = [] + conflicts: list[dict] = [] + + for change in changes: + path = fs.resolve_path(change.path, cwd=cwd) + + if change.type == "add": + if await fs.exists(path): + conflicts.append({"path": path, "reason": "file already exists"}) + continue + staged.append( + StagedPatchChange( + type="add", + path=path, + new_content=change.content or "", + ) + ) + continue + + if change.type == "delete": + if not await fs.isfile(path): + conflicts.append({"path": path, "reason": "file not found"}) + continue + staged.append(StagedPatchChange(type="delete", path=path)) + continue + + if change.type == "update": + if not await fs.isfile(path): + conflicts.append({"path": path, "reason": "file not found"}) + continue + try: + content = await fs.read_text(path) + except UnicodeDecodeError: + conflicts.append({"path": path, "reason": "file is not valid UTF-8 text"}) + continue + except OSError as e: + conflicts.append({"path": path, "reason": str(e)}) + continue + + next_search_start = 0 + failed = False + for chunk in change.chunks: + replaced = _replace_chunk_once(content, chunk, next_search_start) + if replaced is None: + conflicts.append({"path": path, "reason": "old content not found"}) + failed = True + break + content, next_search_start = replaced + if failed: + continue + + move_path = fs.resolve_path(change.move_path, cwd=cwd) if change.move_path else None + if move_path and move_path != path and await fs.exists(move_path): + conflicts.append({"path": move_path, "reason": "move destination already exists"}) + continue + staged.append( + StagedPatchChange( + type="update", + path=path, + move_path=move_path, + new_content=content, + ) + ) + continue + + conflicts.append({"path": path, "reason": f"unsupported change type: {change.type}"}) + + return staged, conflicts + + +async def commit_staged_patch(staged: list[StagedPatchChange], fs: UserFS): + for change in staged: + if change.type == "delete": + await fs.remove(change.path) + continue + + target = change.move_path or change.path + await fs.write(target, change.new_content or "") + if change.move_path and change.move_path != change.path: + await fs.remove(change.path) diff --git a/tests/test_tool_contract.py b/tests/test_tool_contract.py new file mode 100644 index 0000000..523644d --- /dev/null +++ b/tests/test_tool_contract.py @@ -0,0 +1,271 @@ +import os +from pathlib import Path + +os.environ.setdefault("OPEN_TERMINAL_API_KEY", "test-token") + +from fastapi.testclient import TestClient + +from open_terminal.main import app + + +AUTH_HEADERS = {"Authorization": "Bearer test-token"} + + +def _client() -> TestClient: + return TestClient(app) + + +def _operation_ids(client: TestClient) -> set[str]: + schema = client.get("/openapi.json").json() + operation_ids: set[str] = set() + for path_item in schema["paths"].values(): + for operation in path_item.values(): + if isinstance(operation, dict) and "operationId" in operation: + operation_ids.add(operation["operationId"]) + return operation_ids + + +def _operations_by_id(client: TestClient) -> dict[str, dict]: + schema = client.get("/openapi.json").json() + operations: dict[str, dict] = {} + for path_item in schema["paths"].values(): + for operation in path_item.values(): + if isinstance(operation, dict) and "operationId" in operation: + operations[operation["operationId"]] = operation + return operations + + +def _parameter_description(operation: dict, name: str) -> str: + for parameter in operation.get("parameters", []): + if parameter.get("name") == name: + return parameter.get("description") or "" + return "" + + +def test_openapi_exposes_only_core_model_tools(): + client = _client() + + assert _operation_ids(client) == { + "get_environment", + "run_command", + "get_process_status", + "list_processes", + "send_process_input", + "kill_process", + "read_file", + "write_file", + "apply_patch", + "display_file", + } + + +def test_model_visible_tool_descriptions_are_explicit(): + client = _client() + schema = client.get("/openapi.json").json() + operations = _operations_by_id(client) + + run_description = operations["run_command"]["description"].lower() + assert "poll get_process_status" in run_description + assert "process_id" in run_description + assert "relative paths resolve against the session cwd" in run_description + + write_description = operations["write_file"]["description"].lower() + assert "409" in write_description + assert "overwrite=true" in write_description + write_schema = schema["components"]["schemas"]["WriteRequest"]["properties"] + assert "defaults to false" in write_schema["overwrite"]["description"].lower() + + assert "*** Begin Patch" in operations["apply_patch"]["description"] + assert "*** Update File:" in operations["apply_patch"]["description"] + assert "dry_run=true" in operations["apply_patch"]["description"] + patch_schema = schema["components"]["schemas"]["ApplyPatchRequest"]["properties"] + assert "*** End Patch" in patch_schema["patch"]["description"] + + assert "process_id returned by run_command" in _parameter_description( + operations["get_process_status"], "process_id" + ) + assert "process_id returned by run_command" in _parameter_description( + operations["send_process_input"], "process_id" + ) + assert "process_id returned by run_command" in _parameter_description( + operations["kill_process"], "process_id" + ) + assert "literal escape sequences" in operations["send_process_input"]["description"].lower() + + +def test_model_visible_tools_publish_response_shapes(): + client = _client() + schema = client.get("/openapi.json").json() + operations = _operations_by_id(client) + + def response_ref(tool: str, status: str = "200") -> str: + response_schema = operations[tool]["responses"][status]["content"]["application/json"]["schema"] + return response_schema["$ref"].rsplit("/", 1)[-1] + + environment = schema["components"]["schemas"][response_ref("get_environment")] + for field in [ + "os", + "hostname", + "user", + "home", + "cwd", + "shell", + "environment", + "cli_versions", + "permissions", + "info", + ]: + assert field in environment["properties"] + + read_response = operations["read_file"]["responses"]["200"] + assert "application/json" in read_response["content"] + assert "image/png" in read_response["content"] + assert "image/jpeg" in read_response["content"] + assert "Raw binary" in read_response["description"] + + command_response = schema["components"]["schemas"][response_ref("run_command")] + for field in ["id", "command", "status", "exit_code", "output", "truncated", "next_offset", "log_path"]: + assert field in command_response["properties"] + + status_response = schema["components"]["schemas"][response_ref("get_process_status")] + assert status_response["properties"].keys() == command_response["properties"].keys() + + apply_response = schema["components"]["schemas"][response_ref("apply_patch")] + for field in ["applied", "dry_run", "changes", "conflicts"]: + assert field in apply_response["properties"] + apply_conflict_ref = operations["apply_patch"]["responses"]["409"]["content"]["application/json"]["schema"]["$ref"].rsplit("/", 1)[-1] + assert "detail" in schema["components"]["schemas"][apply_conflict_ref]["properties"] + write_conflict_ref = operations["write_file"]["responses"]["409"]["content"]["application/json"]["schema"]["$ref"].rsplit("/", 1)[-1] + assert schema["components"]["schemas"][write_conflict_ref]["properties"]["detail"]["type"] == "string" + patch_schema = schema["components"]["schemas"]["ApplyPatchRequest"]["properties"]["patch"] + assert "*** Begin Patch" in str(patch_schema.get("examples")) + + +def test_get_environment_returns_runtime_and_cli_contract(): + client = _client() + + response = client.get("/environment", headers=AUTH_HEADERS) + + assert response.status_code == 200 + data = response.json() + assert data["os"]["system"] + assert data["hostname"] + assert data["user"] + assert data["home"] + assert data["cwd"] + assert data["shell"] + assert "PATH" in data["environment"] + for name in [ + "rg", + "git", + "jq", + "python3", + "node", + "curl", + "tar", + "zip", + "unzip", + "find", + "sed", + "awk", + "file", + "patch", + "diff", + ]: + assert name in data["cli_versions"] + assert "available" in data["cli_versions"][name] + + +def test_write_file_does_not_overwrite_by_default(tmp_path: Path): + client = _client() + target = tmp_path / "note.txt" + + first = client.post( + "/files/write", + headers=AUTH_HEADERS, + json={"path": str(target), "content": "first"}, + ) + second = client.post( + "/files/write", + headers=AUTH_HEADERS, + json={"path": str(target), "content": "second"}, + ) + overwrite = client.post( + "/files/write", + headers=AUTH_HEADERS, + json={"path": str(target), "content": "second", "overwrite": True}, + ) + + assert first.status_code == 200 + assert second.status_code == 409 + assert second.json()["detail"] == "File already exists; set overwrite=true to replace it" + assert overwrite.status_code == 200 + assert target.read_text() == "second" + + +def test_apply_patch_supports_multi_hunk_dry_run_and_apply(tmp_path: Path): + client = _client() + target = tmp_path / "demo.txt" + target.write_text("alpha\nbeta\ngamma\n") + patch = f"""*** Begin Patch +*** Update File: {target} +@@ +-alpha ++ALPHA +@@ +-gamma ++GAMMA +*** End Patch""" + + dry_run = client.post( + "/files/apply_patch", + headers=AUTH_HEADERS, + json={"patch": patch, "dry_run": True}, + ) + + assert dry_run.status_code == 200 + assert dry_run.json()["applied"] is False + assert dry_run.json()["dry_run"] is True + assert target.read_text() == "alpha\nbeta\ngamma\n" + + applied = client.post( + "/files/apply_patch", + headers=AUTH_HEADERS, + json={"patch": patch}, + ) + + assert applied.status_code == 200 + assert applied.json()["applied"] is True + assert target.read_text() == "ALPHA\nbeta\nGAMMA\n" + + +def test_apply_patch_reports_conflicts(tmp_path: Path): + client = _client() + target = tmp_path / "demo.txt" + target.write_text("alpha\n") + patch = f"""*** Begin Patch +*** Update File: {target} +@@ +-missing ++present +*** End Patch""" + + response = client.post( + "/files/apply_patch", + headers=AUTH_HEADERS, + json={"patch": patch}, + ) + + assert response.status_code == 409 + detail = response.json()["detail"] + assert detail["message"] == "Patch could not be applied" + assert detail["conflicts"][0]["path"] == str(target) + assert detail["conflicts"][0]["reason"] == "old content not found" + assert target.read_text() == "alpha\n" + + +def test_full_dockerfile_installs_required_cli_contract(): + dockerfile = Path("Dockerfile").read_text() + + for package in ["ripgrep", "git", "jq", "nodejs", "curl", "zip", "unzip", "tar", "findutils", "sed", "gawk", "file", "patch", "diffutils"]: + assert package in dockerfile From e99fce1ce9186eff249ef1a226814cabd0d572c9 Mon Sep 17 00:00:00 2001 From: shuofang <916931057@qq.com> Date: Wed, 17 Jun 2026 14:59:00 +0800 Subject: [PATCH 2/2] Clarify agent-facing tool descriptions --- open_terminal/main.py | 167 +++++++++++++++++++++++++----------- tests/test_tool_contract.py | 34 +++++++- 2 files changed, 151 insertions(+), 50 deletions(-) diff --git a/open_terminal/main.py b/open_terminal/main.py index 6bb2bed..3b8f8e9 100644 --- a/open_terminal/main.py +++ b/open_terminal/main.py @@ -151,14 +151,22 @@ def get_system_prompt() -> str: _EXECUTE_DESCRIPTION = ( - "Run a shell command in the background and return a process_id. " - "Use this as the primary system tool for filesystem search, git, package, " - "build, test, and diagnostic commands. Relative paths resolve against the " - "session cwd, or the supplied cwd when provided. If wait is omitted or the " - "command is still running, poll get_process_status with the returned " - "process_id to read output and completion status. Use send_process_input " - "for interactive stdin and kill_process to terminate long-running commands.\n\n" - + get_system_info() + "Run a shell command as a tracked background process and return a process_id.\n\n" + "Use when: you need the primary system primitive for filesystem search, git, " + "package management, builds, tests, diagnostics, or CLI tools not exposed as " + "dedicated tools. Use get_environment first when you need OS, PATH, shell, " + "permission, or CLI availability details.\n" + "Inputs: command is the shell command string; cwd optionally sets the working " + "directory; env optionally adds environment variables; wait controls how long " + "to wait for completion; tail limits returned output entries. Relative paths " + "resolve against the session cwd, or the supplied cwd when provided. Omit or " + "set wait=null to use server default behavior; set wait=0 to return immediately.\n" + "Returns: JSON with id/process_id, command, status, exit_code, output entries, " + "truncated, next_offset, and log_path. If the command is still running, poll " + "get_process_status with the process_id and next_offset.\n" + "Errors: 401 means authentication failed; 422 means request validation failed. " + "Use send_process_input for interactive stdin and kill_process to terminate " + "long-running commands." ) if EXECUTE_DESCRIPTION: _EXECUTE_DESCRIPTION += "\n\n" + EXECUTE_DESCRIPTION @@ -581,10 +589,15 @@ async def get_config(): operation_id="get_environment", summary="Get runtime environment", description=( - "Return stable runtime metadata: OS, hostname, user, home, cwd, shell, " - "PATH, key CLI versions, and permission boundaries. Returns JSON fields: " - "os, hostname, user, home, cwd, shell, environment, cli_versions, " - "permissions, and info." + "Inspect the runtime environment and stable CLI contract.\n\n" + "Use when: starting a task, before choosing OS-specific commands, when a " + "command depends on PATH or shell behavior, or when you need permission and " + "sandbox boundaries.\n" + "Inputs: none.\n" + "Returns: JSON fields os, hostname, user, home, cwd, shell, environment, " + "cli_versions, permissions, and info. Each cli_versions entry reports " + "available, path, and version.\n" + "Errors: 401 means authentication failed." ), response_model=EnvironmentResponse, dependencies=[Depends(verify_api_key)], @@ -723,10 +736,18 @@ async def list_files( operation_id="read_file", summary="Read a file", description=( - "Read a file and return its contents. Text files and extracted documents " - "return JSON with path, total_lines, and content. For text files you can " - "request a line range. Supported images return raw HTTP binary data with " - "the image MIME type, not base64. Use display_file to show a file to the user." + "Read file content for agent analysis.\n\n" + "Use when: you need to inspect text, a line range, extracted document text, " + "or a supported image. For large text files, request start_line and end_line " + "or use run_command with rg/sed/head/tail to avoid excessive context.\n" + "Inputs: path is absolute or relative to the session cwd; start_line and " + "end_line are optional 1-indexed inclusive bounds for text/document content.\n" + "Returns: text files and extracted documents return JSON with path, " + "total_lines, and content. Supported images return raw HTTP binary data " + "with the image MIME type, not base64. Use display_file to show a file to " + "the user.\n" + "Errors: 404 means file not found; 415 means unsupported binary content; " + "401 means authentication failed." ), response_model=ReadFileResponse, dependencies=[Depends(verify_api_key)], @@ -813,10 +834,14 @@ async def read_file( operation_id="display_file", summary="Display a file to the user", description=( - "Make a file visible to the user in the client preview/viewer. Use this " - "when the user wants to view or preview a file. This does not return file " - "content to you; use read_file if you need to read the content yourself. " - "Returns JSON with path and exists." + "Make a file visible to the user in the client preview/viewer.\n\n" + "Use when: the user asks to view, preview, open, or inspect a generated " + "artifact directly. This is a UI/preview signal, not a content-reading tool.\n" + "Inputs: path is the absolute path to display.\n" + "Returns: JSON with path and exists. This does not return file content to " + "you; use read_file if you need to read the content yourself.\n" + "Errors: 401 means authentication failed; 422 means the path parameter was " + "missing or invalid." ), response_model=DisplayFileResponse, dependencies=[Depends(verify_api_key)], @@ -884,11 +909,16 @@ async def serve_file(path: str, fs: UserFS = Depends(get_filesystem)): operation_id="write_file", summary="Write a file", description=( - "Write complete text content to a file. Parent directories are created " - "automatically. By default this tool does not overwrite existing files: " - "an existing path returns HTTP 409. Set overwrite=true only when replacing " - "the whole file is intended. Prefer apply_patch for localized edits. " - "Returns JSON with path and size." + "Write complete text content to a file.\n\n" + "Use when: creating a new text file or intentionally replacing the whole " + "file. Prefer apply_patch for localized edits to existing files.\n" + "Inputs: path is absolute or relative to the session cwd; content is the " + "complete UTF-8 text to write; overwrite defaults to false. Parent " + "directories are created automatically.\n" + "Returns: JSON with path and size in UTF-8 bytes.\n" + "Errors: 409 means the file already exists and overwrite=false; retry with " + "overwrite=true only when replacing the whole file is intended. 401 means " + "authentication failed; 422 means request validation failed." ), response_model=WriteFileResponse, dependencies=[Depends(verify_api_key)], @@ -907,7 +937,11 @@ async def write_file(http_request: Request, request: WriteRequest, fs: UserFS = if not request.overwrite and await fs.exists(target): raise HTTPException( status_code=409, - detail="File already exists; set overwrite=true to replace it", + detail=( + "File already exists and overwrite=false. Retry with overwrite=true " + "only when replacing the whole file is intended; otherwise use " + "apply_patch for localized edits." + ), ) try: await fs.write(target, request.content) @@ -1042,12 +1076,18 @@ async def replace_file_content(http_request: Request, request: ReplaceRequest, f operation_id="apply_patch", summary="Apply a patch", description=( - "Apply an OpenAI apply_patch-format patch. The patch must use markers like " - "'*** Begin Patch', '*** Add File:', '*** Update File:', '*** Delete File:', " - "'*** Move to:', and '*** End Patch'. Supports multi-hunk updates, add, " - "delete, move, dry_run=true validation without writing, and structured " - "409 conflict reports when old content does not match. Returns JSON with " - "applied, dry_run, changes, and conflicts." + "Apply an OpenAI apply_patch-format patch to one or more files.\n\n" + "Use when: making localized file edits, adding files, deleting files, or " + "moving files while preserving conflict detection. Use dry_run=true before " + "risky or multi-file edits.\n" + "Inputs: patch must start with '*** Begin Patch', contain hunks such as " + "'*** Add File:', '*** Update File:', '*** Delete File:', or '*** Move to:', " + "and end with '*** End Patch'. dry_run defaults to false. Example: " + "'*** Begin Patch\\n*** Update File: path/to/file.py\\n@@\\n-old\\n+new\\n*** End Patch'.\n" + "Returns: JSON with applied, dry_run, changes, and conflicts.\n" + "Errors: 400 means patch syntax or commit failed; 409 means the patch did " + "not apply cleanly. On 409, read_file the affected path, rebuild the patch " + "from current content, and retry." ), response_model=ApplyPatchResponse, dependencies=[Depends(verify_api_key)], @@ -1078,7 +1118,10 @@ async def apply_patch( raise HTTPException( status_code=409, detail={ - "message": "Patch could not be applied", + "message": ( + "Patch could not be applied. Re-read the affected file with " + "read_file, rebuild the patch from current content, and retry." + ), "conflicts": conflicts, }, ) @@ -1440,10 +1483,15 @@ def _build_zip() -> bytes: @app.get( "/execute", operation_id="list_processes", - summary="List running commands", + summary="List tracked commands", description=( - "Returns a JSON list of all tracked background processes, including " - "id, command, status, exit_code, and log_path." + "List tracked commands started by run_command.\n\n" + "Use when: you need to find active or recently finished process_ids before " + "polling, sending input, or terminating a command.\n" + "Inputs: none.\n" + "Returns: JSON list of tracked commands with id, command, status, " + "exit_code, and log_path. Status is running, done, or killed.\n" + "Errors: 401 means authentication failed." ), response_model=list[ProcessSummaryResponse], dependencies=[Depends(verify_api_key)], @@ -1481,7 +1529,7 @@ async def execute( request: ExecRequest, wait: Optional[float] = Query( None, - description="Seconds to wait for the command to finish before returning. If the command completes in time, output is included inline. Null to return immediately.", + description="Seconds to wait for completion before returning. Omit or set null to use server default behavior; set 0 to return immediately.", ge=0, le=300, ), @@ -1540,9 +1588,17 @@ async def execute( operation_id="get_process_status", summary="Get command status and output", description=( - "Poll a process started by run_command. Returns output, status, exit_code, " - "next_offset, truncation state, and log_path. Use offset=next_offset from " - "the previous response to read only new output." + "Poll status and output for a process started by run_command.\n\n" + "Use when: run_command returned while the command was still running, or you " + "need more output from a tracked process.\n" + "Inputs: process_id must be the id returned by run_command; offset should " + "usually be the previous next_offset; wait controls how long to wait; tail " + "limits returned output entries.\n" + "Returns: JSON with id/process_id, command, status, exit_code, output, " + "truncated, next_offset, and log_path. Use offset=next_offset to read only " + "new output next time.\n" + "Errors: 404 means the process_id is unknown or expired; 401 means " + "authentication failed; 422 means request validation failed." ), response_model=ProcessStatusResponse, dependencies=[Depends(verify_api_key)], @@ -1558,7 +1614,7 @@ async def get_status( ), wait: Optional[float] = Query( None, - description="Seconds to wait for the process to finish before returning. Returns early if the process exits. Null to return immediately.", + description="Seconds to wait for the process to finish. Omit or set null to use server default behavior; set 0 to return immediately.", ge=0, le=300, ), @@ -1606,9 +1662,16 @@ async def get_status( operation_id="send_process_input", summary="Send input to a running command", description=( - "Write text to stdin for a running process started by run_command. Include " - "newlines when the command expects Enter. Literal escape sequences such as " - "\\n, \\x03, and \\x04 are converted before sending." + "Write text to stdin for a running process started by run_command.\n\n" + "Use when: an interactive command is waiting for input, confirmation, " + "password text, Ctrl-C, or EOF.\n" + "Inputs: process_id must be the id returned by run_command; input is text " + "to send. Include newlines when the command expects Enter. Literal escape " + "sequences such as \\n, \\x03, and \\x04 are converted before sending.\n" + "Returns: JSON with status='ok' after input is accepted.\n" + "Errors: 404 means the process_id is unknown or expired; 400 means the " + "process exited or stdin is closed; 401 means authentication failed; 422 " + "means request validation failed." ), response_model=StatusResponse, dependencies=[Depends(verify_api_key)], @@ -1648,9 +1711,17 @@ async def send_input( operation_id="kill_process", summary="Kill a running command", description=( - "Terminate the process. Sends SIGTERM by default for graceful shutdown " - "on Unix-like backends; use force=true to request SIGKILL or the closest " - "available forceful termination behavior." + "Terminate a process started by run_command.\n\n" + "Use when: a tracked command is hung, no longer needed, or must be stopped " + "before continuing.\n" + "Inputs: process_id must be the id returned by run_command; force=false " + "requests graceful termination, while force=true requests forceful " + "termination.\n" + "Returns: JSON with status='killed'.\n" + "Errors: 404 means the process_id is unknown or expired; 401 means " + "authentication failed; 422 means request validation failed. On Unix-like " + "backends force=false sends SIGTERM and force=true sends SIGKILL; other " + "platforms use the closest available behavior." ), response_model=StatusResponse, dependencies=[Depends(verify_api_key)], @@ -1664,7 +1735,7 @@ async def kill_process( ..., description="The process_id returned by run_command.", ), - force: bool = Query(False, description="Send SIGKILL instead of SIGTERM."), + force: bool = Query(False, description="Request forceful termination instead of graceful termination."), ): background_process = _get_process(process_id) if background_process.status == "running": diff --git a/tests/test_tool_contract.py b/tests/test_tool_contract.py index 523644d..2274989 100644 --- a/tests/test_tool_contract.py +++ b/tests/test_tool_contract.py @@ -64,23 +64,48 @@ def test_model_visible_tool_descriptions_are_explicit(): schema = client.get("/openapi.json").json() operations = _operations_by_id(client) + for operation in operations.values(): + description = operation["description"] + assert "Use when:" in description + assert "Inputs:" in description + assert "Returns:" in description + assert "Errors:" in description + + environment_description = operations["get_environment"]["description"].lower() + assert "before choosing os-specific commands" in environment_description + assert "cli_versions" in environment_description + + read_description = operations["read_file"]["description"].lower() + assert "large text files" in read_description + assert "start_line" in read_description + assert "raw http binary data" in read_description + run_description = operations["run_command"]["description"].lower() assert "poll get_process_status" in run_description assert "process_id" in run_description assert "relative paths resolve against the session cwd" in run_description + assert "get_environment" in run_description + assert "this system is running" not in run_description write_description = operations["write_file"]["description"].lower() assert "409" in write_description assert "overwrite=true" in write_description + assert "whole file" in write_description write_schema = schema["components"]["schemas"]["WriteRequest"]["properties"] assert "defaults to false" in write_schema["overwrite"]["description"].lower() assert "*** Begin Patch" in operations["apply_patch"]["description"] assert "*** Update File:" in operations["apply_patch"]["description"] assert "dry_run=true" in operations["apply_patch"]["description"] + assert "on 409" in operations["apply_patch"]["description"].lower() + assert "read_file" in operations["apply_patch"]["description"] patch_schema = schema["components"]["schemas"]["ApplyPatchRequest"]["properties"] assert "*** End Patch" in patch_schema["patch"]["description"] + list_description = operations["list_processes"]["description"].lower() + assert "tracked commands" in list_description + assert "running, done, or killed" in list_description + assert "process_id returned by run_command" in _parameter_description( operations["get_process_status"], "process_id" ) @@ -198,7 +223,10 @@ def test_write_file_does_not_overwrite_by_default(tmp_path: Path): assert first.status_code == 200 assert second.status_code == 409 - assert second.json()["detail"] == "File already exists; set overwrite=true to replace it" + conflict_detail = second.json()["detail"] + assert "overwrite=false" in conflict_detail + assert "overwrite=true" in conflict_detail + assert "apply_patch" in conflict_detail assert overwrite.status_code == 200 assert target.read_text() == "second" @@ -258,7 +286,9 @@ def test_apply_patch_reports_conflicts(tmp_path: Path): assert response.status_code == 409 detail = response.json()["detail"] - assert detail["message"] == "Patch could not be applied" + assert "Patch could not be applied" in detail["message"] + assert "read_file" in detail["message"] + assert "retry" in detail["message"] assert detail["conflicts"][0]["path"] == str(target) assert detail["conflicts"][0]["reason"] == "old content not found" assert target.read_text() == "alpha\n"