Skip to content

Commit 27c47f1

Browse files
authored
fix(agent): Agent.load() raised for every agent on a read-only property (kyegomez#2014)
SafeStateManager.load_state assigns every safe-typed key from the state file back onto the object: for key, value in state_dict.items(): if not key.startswith("_") and key not in preserved and SafeLoaderUtils.is_safe_type(value): setattr(obj, key, value) for key, value in preserved.items(): setattr(obj, key, value) create_state_dict reads instance state, which includes class-level read-only properties, so the file carries keys that cannot be written back. Agent has two — `workspace` and `mcp_enabled` — and the first one reached ends the load: AttributeError: property 'workspace' of 'Agent' object has no setter That is not an edge case. Every Agent has both properties, so Agent.load() could not complete for any agent, from either loop. is_settable() skips a key whose class attribute is a property with no setter. Both loops consult it. These are derived values — `workspace` and `mcp_enabled` are computed from state that *is* restorable — so skipping them loses nothing, and the test asserts they still hold their computed value after a load rather than being blanked. Why this went unnoticed: the repo's only save/load round-trip test, TestBasicAgent::test_save_and_load, has errored at setup since 2025-10-21 on a fixture deleted out from under it (kyegomez#2010). It looked like coverage for ten months. tests/structs/test_safe_loading.py is a new file — nothing under tests/ owned safe_loading.py. Six tests: the crash itself, a real scalar round trip (max_loops 9 -> 3, agent_name restored), read-only properties surviving, the premise that Agent still has such properties at all, and FileNotFoundError still propagating. Against unfixed source: 5 failed, 1 passed. Here: 6 passed. tests/structs/test_agent.py failure set is unchanged from master (27=27).
1 parent bcda9a4 commit 27c47f1

2 files changed

Lines changed: 127 additions & 1 deletion

File tree

swarms/structs/safe_loading.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,31 @@ def is_class_instance(obj: Any) -> bool:
3939
and obj_type.__module__ != "builtins"
4040
)
4141

42+
@staticmethod
43+
def is_settable(obj: Any, key: str) -> bool:
44+
"""Can ``key`` actually be assigned on ``obj``?
45+
46+
A read-only ``property`` on the class raises AttributeError on
47+
assignment. ``create_state_dict`` reads instance state and happily
48+
serialises such values, so a state file can carry a key that cannot
49+
be written back — Agent has two, ``workspace`` and ``mcp_enabled``,
50+
which made ``Agent.load()`` raise for every agent.
51+
52+
These are derived values in any case: whatever they should be is
53+
recomputed from the state that *is* restorable, so skipping them
54+
loses nothing.
55+
56+
Args:
57+
obj: Object the state is being loaded into, or its class
58+
key: Attribute name from the state file
59+
60+
Returns:
61+
bool: False only for a class-level property with no setter
62+
"""
63+
owner = obj if isinstance(obj, type) else type(obj)
64+
attr = getattr(owner, key, None)
65+
return not (isinstance(attr, property) and attr.fset is None)
66+
4267
@staticmethod
4368
def is_safe_type(value: Any) -> bool:
4469
"""
@@ -220,12 +245,14 @@ def load_state(obj: Any, file_path: str) -> None:
220245
not key.startswith("_")
221246
and key not in preserved
222247
and SafeLoaderUtils.is_safe_type(value)
248+
and SafeLoaderUtils.is_settable(obj, key)
223249
):
224250
setattr(obj, key, value)
225251

226252
# Restore preserved instances
227253
for key, value in preserved.items():
228-
setattr(obj, key, value)
254+
if SafeLoaderUtils.is_settable(obj, key):
255+
setattr(obj, key, value)
229256

230257
logger.info(
231258
f"Successfully loaded state from: {file_path}"

tests/structs/test_safe_loading.py

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
"""SafeStateManager must be able to load back what it saved.
2+
3+
Agent.load() raised AttributeError for *every* agent: create_state_dict
4+
serialises instance state including class-level read-only properties, and
5+
load_state then tried to setattr them back. Agent has two — `workspace` and
6+
`mcp_enabled` — so the very first one hit ended the load.
7+
8+
Nothing caught it because the only save/load round-trip test in the repo,
9+
TestBasicAgent::test_save_and_load, has errored at setup since 2025-10-21
10+
on a fixture that was deleted out from under it (#2010).
11+
12+
Offline: the LLM is a stub, and the workspace is a tmp_path.
13+
"""
14+
15+
import os
16+
17+
import pytest
18+
19+
from swarms import Agent
20+
from swarms.structs.safe_loading import SafeLoaderUtils
21+
22+
23+
class _StubLLM:
24+
def run(self, task=None, *args, **kwargs):
25+
return task
26+
27+
28+
def _agent(tmp_path, name, **overrides):
29+
kwargs = dict(
30+
agent_name=name,
31+
llm=_StubLLM(),
32+
max_loops=1,
33+
print_on=False,
34+
verbose=False,
35+
persistent_memory=False,
36+
autosave=False,
37+
workspace_dir=str(tmp_path),
38+
)
39+
kwargs.update(overrides)
40+
return Agent(**kwargs)
41+
42+
43+
def test_agent_has_read_only_properties_in_its_state():
44+
"""The premise: if this ever stops holding, the guard is unneeded."""
45+
read_only = [
46+
name
47+
for name, value in vars(Agent).items()
48+
if isinstance(value, property) and value.fset is None
49+
]
50+
51+
assert read_only, "Agent no longer has read-only properties"
52+
for name in read_only:
53+
assert not SafeLoaderUtils.is_settable(Agent, name)
54+
55+
56+
def test_is_settable_allows_ordinary_attributes():
57+
assert SafeLoaderUtils.is_settable(Agent, "max_loops")
58+
assert SafeLoaderUtils.is_settable(Agent, "agent_name")
59+
60+
61+
def test_save_then_load_does_not_raise(tmp_path):
62+
"""The reported crash: load() blew up on the first read-only property."""
63+
path = str(tmp_path / "state.json")
64+
_agent(tmp_path, "saver").save(path)
65+
66+
assert os.path.exists(path)
67+
68+
# Fails on unfixed code with:
69+
# AttributeError: property 'workspace' of 'Agent' object has no setter
70+
_agent(tmp_path, "loader").load(path)
71+
72+
73+
def test_scalar_state_actually_round_trips(tmp_path):
74+
"""Not just "does not raise" — the saved values come back."""
75+
path = str(tmp_path / "state.json")
76+
_agent(tmp_path, "saver", max_loops=3).save(path)
77+
78+
restored = _agent(tmp_path, "loader", max_loops=9)
79+
restored.load(path)
80+
81+
assert restored.max_loops == 3
82+
assert restored.agent_name == "saver"
83+
84+
85+
def test_read_only_properties_survive_the_load(tmp_path):
86+
"""Skipping them must not blank them out."""
87+
path = str(tmp_path / "state.json")
88+
_agent(tmp_path, "saver").save(path)
89+
90+
restored = _agent(tmp_path, "loader")
91+
before = restored.workspace
92+
restored.load(path)
93+
94+
assert restored.workspace == before
95+
96+
97+
def test_missing_file_still_raises(tmp_path):
98+
with pytest.raises(FileNotFoundError):
99+
_agent(tmp_path, "loader").load(str(tmp_path / "nope.json"))

0 commit comments

Comments
 (0)