mirror of
https://github.com/hansjone/oclaw.git
synced 2026-10-14 09:40:51 +08:00
fix(runtime): harden tool-call pairing and extend run_command timeouts
Prevent invalid tool_calls replay by requiring immediate tool-result adjacency in message building, and raise run_command default and tool-level timeout ceilings to reduce install-command timeout failures. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
6628cd1220
commit
d731288ffa
4 changed files with 26 additions and 7 deletions
|
|
@ -514,11 +514,30 @@ def build_llm_messages(
|
||||||
tool_calls = None
|
tool_calls = None
|
||||||
|
|
||||||
if tool_calls and isinstance(tool_calls, list):
|
if tool_calls and isinstance(tool_calls, list):
|
||||||
# Guard: only include tool_calls if tool results exist later in this trimmed window.
|
# Guard: only include assistant tool_calls when paired tool rows are present.
|
||||||
|
# OpenAI-compatible providers require strict adjacency:
|
||||||
|
# assistant(tool_calls) must be followed immediately by matching tool rows.
|
||||||
want_ids = [str(tc.get("id") or "").strip() for tc in tool_calls if isinstance(tc, dict) and str(tc.get("id") or "").strip()]
|
want_ids = [str(tc.get("id") or "").strip() for tc in tool_calls if isinstance(tc, dict) and str(tc.get("id") or "").strip()]
|
||||||
suffix = tool_ids_after[i] if (i >= 0 and i < len(tool_ids_after)) else set()
|
suffix = tool_ids_after[i] if (i >= 0 and i < len(tool_ids_after)) else set()
|
||||||
if want_ids and any(tid not in suffix for tid in want_ids):
|
if want_ids and any(tid not in suffix for tid in want_ids):
|
||||||
tool_calls = None
|
tool_calls = None
|
||||||
|
# Stronger guard than suffix-presence: verify immediate following block.
|
||||||
|
if tool_calls is not None and want_ids:
|
||||||
|
immediate_ids: set[str] = set()
|
||||||
|
for j in range(i + 1, len(store_messages)):
|
||||||
|
nm = store_messages[j]
|
||||||
|
n_event_type = str(getattr(nm, "event_type", "") or "").strip().lower()
|
||||||
|
# Ignore reasoning-only rows when checking adjacency.
|
||||||
|
if n_event_type == "reasoning":
|
||||||
|
continue
|
||||||
|
n_role = str(getattr(nm, "role", "") or "")
|
||||||
|
if n_role != "tool":
|
||||||
|
break
|
||||||
|
tcid = _tool_call_id_from_tool_row(getattr(nm, "tool_calls", None))
|
||||||
|
if tcid:
|
||||||
|
immediate_ids.add(str(tcid))
|
||||||
|
if any(tid not in immediate_ids for tid in want_ids):
|
||||||
|
tool_calls = None
|
||||||
if tool_calls is None:
|
if tool_calls is None:
|
||||||
out.append(
|
out.append(
|
||||||
_attach_reasoning_content(
|
_attach_reasoning_content(
|
||||||
|
|
|
||||||
|
|
@ -66,7 +66,7 @@ class LocalAdapter:
|
||||||
os.environ[k] = str(value)
|
os.environ[k] = str(value)
|
||||||
return {"ok": True, "key": k, "value": os.environ.get(k), "deleted": False}
|
return {"ok": True, "key": k, "value": os.environ.get(k), "deleted": False}
|
||||||
|
|
||||||
def run_command(self, *, command: str, cwd: str | None = None, timeout: int = 30) -> dict[str, Any]:
|
def run_command(self, *, command: str, cwd: str | None = None, timeout: int = 300) -> dict[str, Any]:
|
||||||
cmd = str(command or "").strip()
|
cmd = str(command or "").strip()
|
||||||
if not cmd:
|
if not cmd:
|
||||||
return {"ok": False, "error_code": "command_required", "error": "command_required"}
|
return {"ok": False, "error_code": "command_required", "error": "command_required"}
|
||||||
|
|
@ -103,7 +103,7 @@ class LocalAdapter:
|
||||||
except Exception:
|
except Exception:
|
||||||
return {"ok": False, "error_code": "disabled", "error": "disabled"}
|
return {"ok": False, "error_code": "disabled", "error": "disabled"}
|
||||||
try:
|
try:
|
||||||
timeout_s = max(1, min(int(timeout or 30), 600))
|
timeout_s = max(1, min(int(timeout or 300), 600))
|
||||||
# run_command never follows adapter cd state.
|
# run_command never follows adapter cd state.
|
||||||
# It only uses explicit cwd; otherwise defaults to data/workspace.
|
# It only uses explicit cwd; otherwise defaults to data/workspace.
|
||||||
workdir = str(resolve_workspace_path(cwd or "data/workspace"))
|
workdir = str(resolve_workspace_path(cwd or "data/workspace"))
|
||||||
|
|
|
||||||
|
|
@ -12,7 +12,7 @@ def run_command_tool() -> ToolSpec:
|
||||||
if not command:
|
if not command:
|
||||||
return {"ok": False, "error_code": "command_required", "error": "command_required"}
|
return {"ok": False, "error_code": "command_required", "error": "command_required"}
|
||||||
cwd = str(args.get("cwd") or "").strip() or None
|
cwd = str(args.get("cwd") or "").strip() or None
|
||||||
timeout = int(args.get("timeout") or 30)
|
timeout = int(args.get("timeout") or 300)
|
||||||
return get_local_adapter().run_command(command=command, cwd=cwd, timeout=timeout)
|
return get_local_adapter().run_command(command=command, cwd=cwd, timeout=timeout)
|
||||||
|
|
||||||
return ToolSpec(
|
return ToolSpec(
|
||||||
|
|
@ -23,7 +23,7 @@ def run_command_tool() -> ToolSpec:
|
||||||
"properties": {
|
"properties": {
|
||||||
"command": {"type": "string", "description": "Shell command to execute."},
|
"command": {"type": "string", "description": "Shell command to execute."},
|
||||||
"cwd": {"type": "string", "description": "Optional working directory."},
|
"cwd": {"type": "string", "description": "Optional working directory."},
|
||||||
"timeout": {"type": "integer", "description": "Timeout in seconds.", "default": 30},
|
"timeout": {"type": "integer", "description": "Timeout in seconds.", "default": 300},
|
||||||
},
|
},
|
||||||
"required": ["command"],
|
"required": ["command"],
|
||||||
"additionalProperties": False,
|
"additionalProperties": False,
|
||||||
|
|
@ -31,7 +31,7 @@ def run_command_tool() -> ToolSpec:
|
||||||
handler=_handler,
|
handler=_handler,
|
||||||
tags=frozenset({"public", "exec"}),
|
tags=frozenset({"public", "exec"}),
|
||||||
risk_level="high",
|
risk_level="high",
|
||||||
timeout_s=90.0,
|
timeout_s=620.0,
|
||||||
read_only=False,
|
read_only=False,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -74,7 +74,7 @@ def test_local_adapter_backend_roundtrip(tmp_path: Path, monkeypatch) -> None:
|
||||||
|
|
||||||
def test_run_command_tool_handler(monkeypatch) -> None:
|
def test_run_command_tool_handler(monkeypatch) -> None:
|
||||||
class _Adapter:
|
class _Adapter:
|
||||||
def run_command(self, *, command: str, cwd: str | None = None, timeout: int = 30):
|
def run_command(self, *, command: str, cwd: str | None = None, timeout: int = 300):
|
||||||
return {"ok": True, "command": command, "cwd": cwd, "timeout": timeout}
|
return {"ok": True, "command": command, "cwd": cwd, "timeout": timeout}
|
||||||
|
|
||||||
monkeypatch.setattr("oclaw.runtime.tools.public.run_command_tool.get_local_adapter", lambda: _Adapter())
|
monkeypatch.setattr("oclaw.runtime.tools.public.run_command_tool.get_local_adapter", lambda: _Adapter())
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue