diff --git a/.github/workflows/first-contributor-welcome.yml b/.github/workflows/first-contributor-welcome.yml index 69c153db8..2d789ec62 100644 --- a/.github/workflows/first-contributor-welcome.yml +++ b/.github/workflows/first-contributor-welcome.yml @@ -7,6 +7,10 @@ on: permissions: contents: read issues: write + # GitHub accepts either Issues or Pull requests write access for PR comments. + # Request both scopes to match the first-time PR workflow and retain a + # supported permission path when GitHub issues the Actions token. + pull-requests: write jobs: welcome: diff --git a/lib/cli/src/crewai_cli/checkpoint_cli.py b/lib/cli/src/crewai_cli/checkpoint_cli.py index 65cdc0e2c..16a6a3056 100644 --- a/lib/cli/src/crewai_cli/checkpoint_cli.py +++ b/lib/cli/src/crewai_cli/checkpoint_cli.py @@ -246,7 +246,7 @@ def _list_json(location: str) -> list[dict[str, Any]]: ): name = os.path.basename(path) try: - with open(path) as f: + with open(path, encoding="utf-8") as f: raw = f.read() meta = _parse_checkpoint_json(raw, source=name) meta["name"] = name @@ -267,7 +267,7 @@ def _info_json_latest(location: str) -> dict[str, Any] | None: if not files: return None path = files[0] - with open(path) as f: + with open(path, encoding="utf-8") as f: raw = f.read() meta = _parse_checkpoint_json(raw, source=os.path.basename(path)) meta["name"] = os.path.basename(path) @@ -278,7 +278,7 @@ def _info_json_latest(location: str) -> dict[str, Any] | None: def _info_json_file(path: str) -> dict[str, Any]: - with open(path) as f: + with open(path, encoding="utf-8") as f: raw = f.read() meta = _parse_checkpoint_json(raw, source=os.path.basename(path)) meta["name"] = os.path.basename(path) diff --git a/lib/cli/src/crewai_cli/update_crew.py b/lib/cli/src/crewai_cli/update_crew.py index 3935e2f5f..db250fda7 100644 --- a/lib/cli/src/crewai_cli/update_crew.py +++ b/lib/cli/src/crewai_cli/update_crew.py @@ -89,7 +89,7 @@ def migrate_pyproject(input_file: str, output_file: str) -> None: lock_file = "poetry.lock" lock_backup = "poetry-old.lock" if os.path.exists(lock_file): - os.rename(lock_file, lock_backup) + os.replace(lock_file, lock_backup) else: pass diff --git a/lib/cli/tests/test_update_crew.py b/lib/cli/tests/test_update_crew.py new file mode 100644 index 000000000..751074df2 --- /dev/null +++ b/lib/cli/tests/test_update_crew.py @@ -0,0 +1,63 @@ +"""Tests for the ``crewai update`` pyproject migration.""" + +from pathlib import Path + +import pytest + +from crewai_cli.update_crew import migrate_pyproject + + +_POETRY_PYPROJECT = """\ +[tool.poetry] +name = "demo" +version = "0.1.0" +description = "demo crew" +authors = ["Demo Author "] + +[tool.poetry.dependencies] +python = ">=3.10,<3.14" +crewai = "^1.0.0" +""" + + +def _write_project(tmp_path: Path) -> None: + """Create a minimal Poetry-style project with a lock file in ``tmp_path``.""" + (tmp_path / "pyproject.toml").write_text(_POETRY_PYPROJECT, encoding="utf-8") + (tmp_path / "poetry.lock").write_text("current lock\n", encoding="utf-8") + + +def test_migrate_pyproject_backs_up_lock_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The migration renames ``poetry.lock`` to ``poetry-old.lock`` and backs up pyproject.""" + monkeypatch.chdir(tmp_path) + _write_project(tmp_path) + + migrate_pyproject("pyproject.toml", "pyproject.toml") + + assert not (tmp_path / "poetry.lock").exists() + assert (tmp_path / "poetry-old.lock").read_text(encoding="utf-8") == ( + "current lock\n" + ) + assert (tmp_path / "pyproject-old.toml").exists() + + +def test_migrate_pyproject_overwrites_existing_lock_backup( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A second ``crewai update`` must replace a stale ``poetry-old.lock``. + + ``os.rename`` refuses to overwrite an existing destination on Windows + (``FileExistsError``) while POSIX replaces it silently, so re-running the + migration used to fail only on Windows. + """ + monkeypatch.chdir(tmp_path) + _write_project(tmp_path) + (tmp_path / "poetry-old.lock").write_text("stale backup\n", encoding="utf-8") + + migrate_pyproject("pyproject.toml", "pyproject.toml") + + assert not (tmp_path / "poetry.lock").exists() + assert (tmp_path / "poetry-old.lock").read_text(encoding="utf-8") == ( + "current lock\n" + ) diff --git a/lib/crewai/src/crewai/llms/providers/azure/completion.py b/lib/crewai/src/crewai/llms/providers/azure/completion.py index eecc1a059..0f45bfcef 100644 --- a/lib/crewai/src/crewai/llms/providers/azure/completion.py +++ b/lib/crewai/src/crewai/llms/providers/azure/completion.py @@ -998,19 +998,25 @@ class AzureCompletion(BaseLLM): if choice.delta and choice.delta.tool_calls: for idx, tool_call in enumerate(choice.delta.tool_calls): - if idx not in tool_calls: - tool_calls[idx] = { + tool_index = tool_call.get("index") + if tool_index is None: + tool_index = idx + + if tool_index not in tool_calls: + tool_calls[tool_index] = { "id": tool_call.id, "name": "", "arguments": "", } - elif tool_call.id and not tool_calls[idx]["id"]: - tool_calls[idx]["id"] = tool_call.id + elif tool_call.id and not tool_calls[tool_index]["id"]: + tool_calls[tool_index]["id"] = tool_call.id if tool_call.function and tool_call.function.name: - tool_calls[idx]["name"] = tool_call.function.name + tool_calls[tool_index]["name"] = tool_call.function.name if tool_call.function and tool_call.function.arguments: - tool_calls[idx]["arguments"] += tool_call.function.arguments + tool_calls[tool_index]["arguments"] += ( + tool_call.function.arguments + ) self._emit_stream_chunk_event( chunk=tool_call.function.arguments @@ -1019,13 +1025,13 @@ class AzureCompletion(BaseLLM): from_task=from_task, from_agent=from_agent, tool_call={ - "id": tool_calls[idx]["id"], + "id": tool_calls[tool_index]["id"], "function": { - "name": tool_calls[idx]["name"], - "arguments": tool_calls[idx]["arguments"], + "name": tool_calls[tool_index]["name"], + "arguments": tool_calls[tool_index]["arguments"], }, "type": "function", - "index": idx, + "index": tool_index, }, call_type=LLMCallType.TOOL_CALL, response_id=response_id, diff --git a/lib/crewai/src/crewai/llms/providers/gemini/completion.py b/lib/crewai/src/crewai/llms/providers/gemini/completion.py index 68238a0e1..6ca6da229 100644 --- a/lib/crewai/src/crewai/llms/providers/gemini/completion.py +++ b/lib/crewai/src/crewai/llms/providers/gemini/completion.py @@ -599,6 +599,14 @@ class GeminiCompletion(BaseLLM): mime_type=inline["mimeType"], ) ) + elif "fileData" in item: + file_data = item["fileData"] + parts.append( + types.Part.from_uri( + file_uri=file_data["fileUri"], + mime_type=file_data["mimeType"], + ) + ) else: parts.append(types.Part.from_text(text=str(item))) else: diff --git a/lib/crewai/src/crewai/memory/unified_memory.py b/lib/crewai/src/crewai/memory/unified_memory.py index dcd5383ce..c0654a2ba 100644 --- a/lib/crewai/src/crewai/memory/unified_memory.py +++ b/lib/crewai/src/crewai/memory/unified_memory.py @@ -147,7 +147,12 @@ class Memory(BaseModel): ) read_only: bool = Field( default=False, - description="If True, remember() and remember_many() are silent no-ops.", + description=( + "If True, stored records are left unchanged: remember() and " + "remember_many() store nothing, update() leaves the record " + "untouched, and recall() does not refresh access times. Explicit " + "deletion through forget()/reset() is unaffected." + ), ) root_scope: str | None = Field( default=None, @@ -781,7 +786,7 @@ class Memory(BaseModel): ) results = flow.state.final_results - if results: + if results and not self.read_only: try: touch = getattr(self._storage, "touch_records", None) if touch is not None: @@ -869,7 +874,8 @@ class Memory(BaseModel): importance: New importance score. Returns: - The updated MemoryRecord. + The updated MemoryRecord, or the unchanged record when ``read_only`` + is set. Raises: ValueError: If the record is not found. @@ -877,6 +883,8 @@ class Memory(BaseModel): existing = self._storage.get_record(record_id) if existing is None: raise ValueError(f"Record not found: {record_id}") + if self.read_only: + return existing now = datetime.utcnow() updates: dict[str, Any] = {"last_accessed": now} if content is not None: diff --git a/lib/crewai/tests/llms/azure/test_azure_streaming_tool_calls.py b/lib/crewai/tests/llms/azure/test_azure_streaming_tool_calls.py new file mode 100644 index 000000000..3eca0f6b9 --- /dev/null +++ b/lib/crewai/tests/llms/azure/test_azure_streaming_tool_calls.py @@ -0,0 +1,81 @@ +"""Regression tests for parallel tool calls in Azure streaming completions. + +Azure AI Inference is OpenAI-compatible: each streamed chunk carries a +``tool_call`` whose own ``index`` field identifies the call the delta belongs +to. The streaming accumulator must key on that wire index, not on the position +of the tool_call inside the chunk, otherwise parallel tool calls collapse into +a single corrupted call. +""" + +import json +from unittest.mock import patch + +from azure.ai.inference.models import StreamingChatCompletionsUpdate + +from crewai.llms.providers.azure.completion import AzureCompletion + + +def _tool_call_chunk(index: int, call_id: str, name: str, arguments: str): + """Build one streaming update carrying a single tool-call delta.""" + return StreamingChatCompletionsUpdate( + id="chatcmpl-1", + model="gpt-4o", + created=None, + choices=[ + { + "index": 0, + "delta": { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "index": index, + "id": call_id, + "type": "function", + "function": {"name": name, "arguments": arguments}, + } + ], + }, + "finish_reason": None, + } + ], + ) + + +def test_azure_streaming_parallel_tool_calls_are_keyed_by_wire_index(): + """Both parallel tool calls must reach the executor intact. + + Each chunk carries exactly one tool_call tagged with its own wire ``index`` + (0 and 1). Aggregating by position inside the chunk puts every delta in + slot 0, so the executor receives a single call holding the first call's id, + the last call's name, and the two argument fragments concatenated into + invalid JSON. + """ + chunks = [ + _tool_call_chunk(0, "call_aaa", "get_weather", '{"city":'), + _tool_call_chunk(0, "call_aaa", "get_weather", '"Paris"}'), + _tool_call_chunk(1, "call_bbb", "get_time", '{"tz":'), + _tool_call_chunk(1, "call_bbb", "get_time", '"UTC"}'), + ] + + llm = AzureCompletion( + model="gpt-4o", + api_key="test-key", + endpoint="https://test.openai.azure.com", + stream=True, + ) + + with patch.object(llm._client, "complete") as mock_complete: + mock_complete.return_value = iter(chunks) + + result = llm.call([{"role": "user", "content": "Weather and time in Paris?"}]) + + assert isinstance(result, list) + assert len(result) == 2 + assert [call["id"] for call in result] == ["call_aaa", "call_bbb"] + + weather, clock = result + assert weather["function"]["name"] == "get_weather" + assert json.loads(weather["function"]["arguments"]) == {"city": "Paris"} + assert clock["function"]["name"] == "get_time" + assert json.loads(clock["function"]["arguments"]) == {"tz": "UTC"} diff --git a/lib/crewai/tests/llms/google/test_google.py b/lib/crewai/tests/llms/google/test_google.py index 71f878513..0b07351e0 100644 --- a/lib/crewai/tests/llms/google/test_google.py +++ b/lib/crewai/tests/llms/google/test_google.py @@ -522,6 +522,71 @@ def test_gemini_message_formatting(): assert formatted_contents[1].role == "model" +@pytest.mark.parametrize( + "file_uri", + [ + "https://storage.googleapis.com/example/image.jpg", + "gs://example-bucket/image.jpg", + ], +) +def test_gemini_message_formatting_preserves_file_data(file_uri): + """Test that Gemini file references are preserved in their original order.""" + llm = LLM(model="google/gemini-2.0-flash-001") + + formatted_contents, _ = llm._format_messages_for_gemini( + [ + { + "role": "user", + "content": [ + {"text": "Before"}, + { + "fileData": { + "fileUri": file_uri, + "mimeType": "image/jpeg", + } + }, + {"text": "After"}, + ], + } + ] + ) + + parts = formatted_contents[0].parts + assert parts[0].text == "Before" + assert parts[1].file_data is not None + assert parts[1].file_data.file_uri == file_uri + assert parts[1].file_data.mime_type == "image/jpeg" + assert parts[2].text == "After" + + +def test_gemini_message_formatting_preserves_file_data_without_text(): + """Test that a message containing only a file reference is preserved.""" + llm = LLM(model="google/gemini-2.0-flash-001") + file_uri = "gs://example-bucket/image.jpg" + + formatted_contents, _ = llm._format_messages_for_gemini( + [ + { + "role": "user", + "content": [ + { + "fileData": { + "fileUri": file_uri, + "mimeType": "image/jpeg", + } + } + ], + } + ] + ) + + parts = formatted_contents[0].parts + assert len(parts) == 1 + assert parts[0].file_data is not None + assert parts[0].file_data.file_uri == file_uri + assert parts[0].file_data.mime_type == "image/jpeg" + + def test_gemini_message_formatting_appends_user_turn_after_trailing_model_turn(): """ Gemini's generateContent API rejects a request whose history ends on a diff --git a/lib/crewai/tests/memory/test_unified_memory.py b/lib/crewai/tests/memory/test_unified_memory.py index 65363efad..a1c4e989d 100644 --- a/lib/crewai/tests/memory/test_unified_memory.py +++ b/lib/crewai/tests/memory/test_unified_memory.py @@ -335,6 +335,65 @@ def test_memory_slice_remember_is_noop_when_read_only(tmp_path: Path, mock_embed assert mem.list_records() == [] +def test_update_is_noop_when_read_only(tmp_path: Path, mock_embedder: MagicMock) -> None: + """A read-only Memory leaves stored records untouched when update() is called.""" + from crewai.memory.unified_memory import Memory + + mem = Memory(storage=str(tmp_path / "db8"), llm=MagicMock(), embedder=mock_embedder) + record = mem.remember( + "original", scope="/a", categories=[], importance=0.5, metadata={} + ) + assert record is not None + + mem.read_only = True + returned = mem.update(record.id, content="rewritten", importance=0.9) + + assert returned.content == "original" + assert returned.importance == 0.5 + stored = mem.list_records() + assert [(r.content, r.importance) for r in stored] == [("original", 0.5)] + + +def test_update_still_writes_when_not_read_only( + tmp_path: Path, mock_embedder: MagicMock +) -> None: + """The read-only guard does not change update() for a writable Memory.""" + from crewai.memory.unified_memory import Memory + + mem = Memory(storage=str(tmp_path / "db9"), llm=MagicMock(), embedder=mock_embedder) + record = mem.remember( + "original", scope="/a", categories=[], importance=0.5, metadata={} + ) + assert record is not None + + returned = mem.update(record.id, content="rewritten", importance=0.9) + + assert returned.content == "rewritten" + assert returned.importance == 0.9 + stored = mem.list_records() + assert [(r.content, r.importance) for r in stored] == [("rewritten", 0.9)] + + +def test_recall_does_not_refresh_access_time_when_read_only( + tmp_path: Path, mock_embedder: MagicMock +) -> None: + """Recall against a read-only Memory leaves last_accessed untouched.""" + from crewai.memory.unified_memory import Memory + + mem = Memory(storage=str(tmp_path / "db10"), llm=MagicMock(), embedder=mock_embedder) + mem.remember("alpha", scope="/a", categories=[], importance=0.5, metadata={}) + before = mem.list_records()[0].last_accessed + + mem.read_only = True + assert mem.recall("alpha", scope="/a", limit=5, depth="shallow") + assert mem.list_records()[0].last_accessed == before + + # Positive control: a writable Memory still refreshes the access time. + mem.read_only = False + assert mem.recall("alpha", scope="/a", limit=5, depth="shallow") + assert mem.list_records()[0].last_accessed > before + + def test_flow_has_default_memory() -> None: diff --git a/lib/crewai/tests/test_checkpoint_cli.py b/lib/crewai/tests/test_checkpoint_cli.py index b0b56b3c6..5007ba557 100644 --- a/lib/crewai/tests/test_checkpoint_cli.py +++ b/lib/crewai/tests/test_checkpoint_cli.py @@ -13,6 +13,9 @@ from unittest.mock import MagicMock, patch import pytest from crewai_cli.checkpoint_cli import ( + _info_json_file, + _info_json_latest, + _list_json, _parse_checkpoint_json, _parse_duration, _prune_json, @@ -23,6 +26,7 @@ from crewai_cli.checkpoint_cli import ( prune_checkpoints, resume_checkpoint, ) +from crewai.state.provider.json_provider import JsonProvider def _make_checkpoint_data( @@ -198,6 +202,46 @@ class TestResolveCheckpoint: assert _resolve_checkpoint("/nonexistent/path", None) is None +class TestNonAsciiJsonCheckpoint: + """JSON checkpoints are UTF-8 on disk; the CLI readers must decode them as such. + + ``JsonProvider`` writes checkpoints with ``encoding="utf-8"`` and the runtime + serialises non-ASCII text verbatim, so readers that rely on the platform + default encoding break on Windows (cp1252) for any non-ASCII checkpoint. + """ + + # "Đ" (U+0110) encodes to 0xC4 0x90; 0x90 is undefined in cp1252, so a + # locale-dependent read fails loudly instead of silently producing mojibake. + _NAME = "Đội ngũ phân tích" + + def _write_checkpoint(self, base_dir: str) -> str: + """Write a UTF-8 checkpoint whose entity name is non-ASCII, as the runtime does.""" + data = json.loads(_make_checkpoint_data(name=self._NAME)) + raw = json.dumps(data, ensure_ascii=False) + return JsonProvider().checkpoint(raw, base_dir, branch="main") + + def test_info_json_file_reads_utf8(self, tmp_path: Any) -> None: + """``_info_json_file`` decodes a non-ASCII checkpoint on any platform.""" + path = self._write_checkpoint(str(tmp_path)) + meta = _info_json_file(path) + assert meta["entities"][0]["name"] == self._NAME + + def test_info_json_latest_reads_utf8(self, tmp_path: Any) -> None: + """``_info_json_latest`` decodes the newest non-ASCII checkpoint.""" + self._write_checkpoint(str(tmp_path)) + meta = _info_json_latest(str(tmp_path)) + assert meta is not None + assert meta["entities"][0]["name"] == self._NAME + + def test_list_json_reads_utf8(self, tmp_path: Any) -> None: + """``_list_json`` lists a non-ASCII checkpoint with its real size and entities.""" + self._write_checkpoint(str(tmp_path)) + results = _list_json(str(tmp_path)) + assert len(results) == 1 + assert results[0]["size"] > 0 + assert results[0]["entities"][0]["name"] == self._NAME + + class TestTaskListFromMeta: def test_flattens_tasks(self) -> None: data = _make_checkpoint_data(tasks_completed=2, tasks_total=3)