Replace eval() with AST-based coercion in CodeAct return_result

[alessiodevoto]

Summary

Fixes an indirect code-execution path in CodeAct's return_result argument coercion.

The issue

When the model calls return_result as a tool with a stringified constructor (e.g. {"result": "Answer(answer=1, reason='ok')"}), _maybe_eval_constructor_string in src/nooa/strategies/codeact.py coerces that string into an actual Answer instance. The previous implementation did this with eval(stripped, {"__builtins__": {}}, session.session_locals).

session.session_locals accumulates every object created by prior execute_python cells (populated from result.captured_locals at codeact.py:1506 and 2446). Stripping __builtins__ blocked open / __import__ etc., but it did not block attribute access or method invocation on session objects. A constructor string like Answer(answer=planted.trigger()) — where planted was any object bound in an earlier cell — would execute planted.trigger() as a side effect of coercion.

Impact: the return_result tool-call surface is nominally a "return a value" boundary, but this made it a second path to arbitrary code execution driven by LLM-controlled strings, distinct from the intended execute_python cell path. Concretely relevant when the payload flows in from a less-trusted layer (e.g. a delegated/subagent result) that a caller might assume is inert.

Fix

Replace eval() with an AST-based coercion:

• Parse via ast.parse(mode="eval") and require a bare Call(func=Name(...)).
• Require call.func.id == return_type.__name__ and isinstance(base_return_type, type) — no fallback to arbitrary session-local classes. Annotated[T, ...] return types are unwrapped via _extract_annotated_description.
• Reject **kwargs unpacking (kw.arg is None); reject nested calls, attribute access, subscripting, and starred args (they fall through to ast.literal_eval which raises).
• Constructor args accept Python literals or Name references to plain-JSON session values only (exact bool/int/float/str/list/tuple/dict, no subclasses). Session values are detached via json.loads(json.dumps(..., allow_nan=False)) before use, so custom __getitem__ / dunder methods on subclassed containers can't leak into return_type(**...).
• Instantiate base_return_type(*args, **kwargs) directly rather than eval'ing source.

Behavior changes

_maybe_eval_constructor_string is a fallback for a documented anti-pattern (LLMs calling return_result as a tool with a stringified constructor instead of return_result(value) from inside execute_python; see the guidance at codeact.py:564). The sanctioned paths are unaffected:

• return_result(literal) — works.
• return_result(var) where var is any session local — works (handled by the variable-ref resolution path at codeact.py:1792, which runs before coercion).
• return_result("ClassName(field=literal, ...)") with only literals or plain-JSON session values — works, values are JSON-copied.

Newly rejected (each falls back to Pydantic validation, which fails and the LLM retries):

• Nested calls in args: Answer(answer=min(3, 1, 2)).
• Attribute access: Answer(reason=obj.description).
• Constructor args referencing non-plain-JSON session values (e.g. Pydantic model instances). Note: passing the model directly via return_result(var) still works — only the "re-wrap in a constructor call" form is blocked.

Test plan

• tests/strategies/test_codeact_pure_python_coverage.py::TestMaybeEvalConstructorString — 12/12 pass, including new regression tests:
  • test_constructor_does_not_call_session_object — planted callable in session_locals is not invoked.
  • test_constructor_rejects_non_plain_session_object — arbitrary object references rejected without triggering __iter__ / __repr__.
  • test_constructor_with_session_value_is_json_detached — plain values pass through and are copied (identity-distinct from the source).
  • test_nested_calls_return_as_is — replaces the old test_nested_parens_work; nested calls are now rejected.
• Full file: 176/176 pass.
• All strategy tests: 893/893 pass.
• Reviewer: confirm no downstream consumers rely on the previously permissive coercion (nested calls, attribute lookups, non-JSON session refs inside constructor args).

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[alessiodevoto]

Closing without merging.

• CodeAct's design already grants the model arbitrary Python execution via execute_python, so hardening the eval() in _maybe_eval_constructor_string doesn't close a real boundary — anything reachable through the coercion path is already reachable through a code cell directly.
• This fix would not allow agents to reference (pass by reference) results when returning results, which is one of NOOA's most important features.
  The right mitigation for the underlying concern is running CodeAct inside openshell (or an equivalent sandbox).