diff --git a/src/agents_shipgate/inputs/openai_sdk_static.py b/src/agents_shipgate/inputs/openai_sdk_static.py index 0c7d424e..7a8f0316 100644 --- a/src/agents_shipgate/inputs/openai_sdk_static.py +++ b/src/agents_shipgate/inputs/openai_sdk_static.py @@ -3,7 +3,7 @@ import ast import json from collections.abc import Callable -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path, PurePosixPath from typing import Any, ClassVar, Literal @@ -74,6 +74,7 @@ guard_module_metadata, read_guard_dependency, ) +from agents_shipgate.inputs.tool_list_candidates import tool_list_candidates from agents_shipgate.schemas.coverage_recovery import CoverageRecovery, SourceRecoveryEvidence from agents_shipgate.schemas.guard_dependencies import GuardDependencyEvidence from agents_shipgate.schemas.manifest import ( @@ -490,6 +491,22 @@ def unread_agent( recovery=CoverageRecovery(kind="unresolved", reason="sdk_tools_expression_unresolved"), )) tools_complete = False + # A refused constructor does not erase the source initializer. + # Retain its readable members only as candidates, with the same + # refusal attached to each tool. No ownership proof is relaxed. + candidates = ( + tool_list_candidates(tools_expr, module, imports.resolver) + if listed.unresolved and constructor_issue is not None else () + ) + if context.invocation is not None: + for condition in context.invocation.conditions: + candidates = tuple( + replace(member, conditions=tuple(dict.fromkeys((condition, *member.conditions)))) + for member in candidates + ) + resolved_nodes = {id(member.expr) for member in listed.members} + candidates = tuple(member for member in candidates if id(member.expr) not in resolved_nodes) + candidate_nodes = {id(member.expr) for member in candidates} # Two different definitions under one tool name: the model # sees one name for both, so neither is bound (#879 review). duplicated: set[str] = set() @@ -561,7 +578,7 @@ def bind_object( when.add(name, conditions) return True - for member in listed.members: + for member in (*listed.members, *candidates): element = member.expr home = member.module if member.module is not None else module object_found = ( @@ -570,6 +587,8 @@ def bind_object( else None ) if isinstance(object_found, ObjectBinding): + if id(member.expr) in candidate_nodes: + continue # This initializer fallback covers function references only. if not bind_object(object_found, member.conditions, member.invocation): tools_complete = False continue @@ -710,6 +729,12 @@ def bind_object( ) warnings.append(reason) tool_issues[tool.name] = reason + if id(member.expr) in candidate_nodes: + reason = ( + f"The source initializer names {tool.name!r}, but its binding is not established: " + f"{constructor_issue}; {unread_list_reason('OpenAI Agents SDK', target, pointer, listed)}" + ) + tool_issues[tool.name] = "; ".join(filter(None, (tool_issues.get(tool.name), reason))) handoff_list = lists.resolve(_keyword(call, "handoffs"), invocation=context.invocation) handoffs_complete = not context.limits and constructor_issue is None if lists.construction_changed(context.invocation, "handoffs"): diff --git a/src/agents_shipgate/inputs/tool_list_candidates.py b/src/agents_shipgate/inputs/tool_list_candidates.py new file mode 100644 index 00000000..c4a624d3 --- /dev/null +++ b/src/agents_shipgate/inputs/tool_list_candidates.py @@ -0,0 +1,103 @@ +"""Source initializer hints for an unread SDK tools list. + +These members are candidates only. This walk does not prove ownership, +immutability, constructor identity or the resulting agent's tool surface. +The SDK reader must attach the original refusal to every member it uses. +""" + +from __future__ import annotations + +import ast + +from agents_shipgate.inputs.list_expressions import ( + MAX_DEPTH, + MAX_MEMBERS, + MAX_VISITS, + ListMember, + bindings_at, +) +from agents_shipgate.inputs.python_imports import ( + ImportResolver, + PythonModule, + ScopeIndex, + reference_spelling, +) + + +def tool_list_candidates( + expression: ast.expr | None, module: PythonModule | None, resolver: ImportResolver, +) -> tuple[ListMember, ...]: + """Read bounded literal initializers without granting a binding fact. + + Calls, caller parameters, ambiguous assignments and computed attributes + remain unread. Imports use the resolver's captured source; a refused + import with no initializer supplies no candidate. + """ + visits = 0 + member_count = 0 + exhausted = False + scopes: dict[int, ScopeIndex] = {} + + def walk(node: ast.expr, home: PythonModule, depth: int, seen: frozenset) -> list[ListMember]: + nonlocal visits, member_count, exhausted + if exhausted: + return [] + visits += 1 + if depth > MAX_DEPTH or visits > MAX_VISITS: + exhausted = True + return [] + if isinstance(node, ast.List | ast.Tuple): + members = [] + for item in node.elts: + if isinstance(item, ast.Starred): + members.extend(walk(item.value, home, depth + 1, seen)) + elif isinstance(item, ast.Name | ast.Attribute): + member_count += 1 + if member_count > MAX_MEMBERS: + exhausted = True + return [] + members.append(ListMember(item, home)) + if exhausted: + return [] + return members + if isinstance(node, ast.BinOp) and isinstance(node.op, ast.Add): + return walk(node.left, home, depth + 1, seen) + walk(node.right, home, depth + 1, seen) + if isinstance(node, ast.IfExp): + if isinstance(node.test, ast.Constant): + return walk(node.body if node.test.value else node.orelse, home, depth + 1, seen) + condition = ast.unparse(node.test) + return [ + ListMember(member.expr, member.module, (label, *member.conditions)) + for branch, label in ((node.body, f"`{condition}`"), (node.orelse, f"not `{condition}`")) + for member in walk(branch, home, depth + 1, seen) + ] + spelling = reference_spelling(node) + if spelling is None or not isinstance(node, ast.Name | ast.Attribute): + return [] + index = scopes.get(id(home.tree)) + if index is None: + index = scopes[id(home.tree)] = ScopeIndex(home.tree) + found = bindings_at(index, home.bindings)(spelling.partition(".")[0], node) + if len(found) != 1: + return [] + binding, statement = found[0] + key = (id(home.tree), id(binding)) + if key in seen: + return [] + if isinstance(binding, ast.alias) and isinstance(statement, ast.Import | ast.ImportFrom): + result = resolver.resolve_local_import(home, statement, binding, spelling) + if result.value is None or result.module is None: + return [] + return walk(result.value, result.module, depth + 1, seen | {key}) + if ( + isinstance(node, ast.Name) and isinstance(statement, ast.Assign | ast.AnnAssign) + and statement.value is not None + and (statement.targets if isinstance(statement, ast.Assign) else [statement.target]) == [binding] + ): + return walk(statement.value, home, depth + 1, seen | {key}) + return [] + + if expression is None or module is None: + return () + members = walk(expression, module, 0, frozenset()) + return () if exhausted else tuple(members) diff --git a/tests/shard_seconds.json b/tests/shard_seconds.json index 436348ce..f4f5f810 100644 --- a/tests/shard_seconds.json +++ b/tests/shard_seconds.json @@ -54,6 +54,7 @@ "tests/test_anthropic_api.py": 0.4, "tests/test_application_diff.py": 61.0, "tests/test_application_diff_identity.py": 22.6, + "tests/test_application_diff_list_candidates.py": 7.4, "tests/test_application_diff_reach.py": 66.6, "tests/test_application_diff_review.py": 48.8, "tests/test_application_diff_tool_reach.py": 87.8, @@ -434,5 +435,18 @@ "report_sha256": "a523a72643fd5daf9a09b7ca0efc6c36a5218b9890a53f7f428f655b04dcb0ab", "source": "junit", "time_basis": "JUnit testcase time; phase accounting is selected by the runner" - } + }, + "supplemental_measurements": [ + { + "source": "junit", + "measured_at": "095c5e59724d with independent review condition fix", + "report_sha256": "10c16e790505b92605aac0f3149c242b2bbabd8049af3bfcc37c4ef172458bf2", + "files": { + "tests/test_application_diff_list_candidates.py": 7.4 + }, + "case_count": 13, + "command": "python -m pytest tests/test_application_diff_list_candidates.py -n 4 --junitxml=candidates-timing.xml", + "time_basis": "JUnit testcase time; original full-suite measurements retained" + } + ] } diff --git a/tests/test_application_diff_list_candidates.py b/tests/test_application_diff_list_candidates.py new file mode 100644 index 00000000..bc334df5 --- /dev/null +++ b/tests/test_application_diff_list_candidates.py @@ -0,0 +1,100 @@ +"""An unread constructor keeps named source evidence without granting rows (#1001).""" + +from __future__ import annotations + +import ast + +import pytest + +from agents_shipgate.inputs.list_expressions import MAX_MEMBERS +from agents_shipgate.inputs.python_imports import ImportResolver +from agents_shipgate.inputs.tool_list_candidates import tool_list_candidates +from tests.test_imported_tool_bindings import _commit, _compare, _git, _write + +TOOLS = "from agents import function_tool\n@function_tool\ndef read():\n return 'before'\nBASE = [read]\n" + + +@pytest.mark.parametrize("shape", ["imported-list", "returned-handle"]) +def test_unread_list_initializers_keep_changed_tool_candidates(tmp_path, shape): + # CRM's imported NARZEDZIA_WYCENY and Finance's literal spread retain + # inspectable definitions even when a constructor/handle proof refuses. + if shape == "imported-list": + files = { + "agent.py": "from agents import Agent\nfrom tools import BASE\ndef build():\n return Agent(name='Finance', tools=BASE)\n", + "smoke.py": "import importlib, sys\nimportlib.import_module(sys.argv[1])\n", + } + else: + files = { + "agent.py": "from agents import Agent\nfrom tools import BASE\ndef build(handoffs):\n return Agent(name='Finance', tools=[*BASE], handoffs=handoffs)\n", + "caller.py": "from agent import build\nfrom transport import publish\npublish(build([]))\n", + } + files["tools.py"] = TOOLS + _git(tmp_path, "init", "-q", "-b", "main") + base = _commit(tmp_path, files) + head = _commit(tmp_path, {"tools.py": TOOLS.replace("'before'", "'after'")}) + result = _compare(tmp_path, base, head) + + assert result["comparison_status"] == "partial" + (row,) = [row for row in result["rows"] if row["tool"] == "read"] + assert (row["agent"], row["change"], row["candidate_change"]) == ("Finance", "not_established", "changed") + assert row["before"] and row["after"] + assert row["uncertainty"] + assert any("constructor identity is not established" in limit for limit in result["head"]["limits"]) + + +def test_recovered_candidates_keep_caller_and_initializer_conditions(tmp_path): + _git(tmp_path, "init", "-q", "-b", "main") + files = { + "agent.py": "from agents import Agent\nfrom tools import BASE\ndef build(handoffs):\n return Agent(name='Finance', tools=[*BASE] if available else [], handoffs=handoffs)\n", + "caller.py": "from agent import build\nfrom transport import publish\nif enabled:\n publish(build([]))\n", + "tools.py": TOOLS, + } + base = _commit(tmp_path, files) + head = _commit(tmp_path, {"caller.py": files["caller.py"].replace("enabled", "restricted")}) + result = _compare(tmp_path, base, head) + + (row,) = [row for row in result["rows"] if row["tool"] == "read"] + assert row["change"] == "not_established" + assert row["candidate_change"] == "changed" + assert row["before"]["bound_when"] == ["the caller's condition `enabled` holds and `available`"] + assert row["after"]["bound_when"] == ["the caller's condition `restricted` holds and `available`"] + assert row["uncertainty"] + + +def test_mutating_a_literal_list_does_not_grant_an_established_candidate(tmp_path): + _git(tmp_path, "init", "-q", "-b", "main") + files = { + "agent.py": "from agents import Agent\nfrom tools import BASE\ndef build():\n return Agent(name='Finance', tools=BASE)\n", + "smoke.py": "import importlib, sys\nimportlib.import_module(sys.argv[1])\n", + "mutator.py": "from tools import BASE\nBASE.clear()\n", + "tools.py": TOOLS, + } + base = _commit(tmp_path, files) + head = _commit(tmp_path, {"tools.py": TOOLS.replace("'before'", "'after'")}) + result = _compare(tmp_path, base, head) + assert result["comparison_status"] == "partial" + assert all(row["change"] == "not_established" for row in result["rows"]) + assert any("may be changed in place" in limit for limit in result["head"]["limits"]) + + +@pytest.mark.parametrize( + ("source", "expression", "expected"), + [ + ("BASE = [read]\n", "[*BASE] + (extra,)", [("read", ()), ("extra", ())]), + ("", "[read] if enabled else [extra]", [("read", ("`enabled`",)), ("extra", ("not `enabled`",))]), + ("", "[read] if False else [extra]", [("extra", ())]), + ("A = B\nB = A\n", "A", []), + ("BASE = [read]\nBASE = [extra]\n", "BASE", []), + ("", "load_tools()", []), + ("", "[make_tool(), *load_tools(), read]", [("read", ())]), + ("", "[" + ",".join("read" for _ in range(MAX_MEMBERS + 1)) + "]", []), + ("BASE = [" + ",".join("read" for _ in range(MAX_MEMBERS)) + "]\n", "BASE + BASE", []), + ], + ids=["spread-concatenation", "conditions", "constant-condition", "cycle", "rebound", "call", "unread-operands", "member-bound", "shared-member-bound"], +) +def test_candidate_initializer_walk_has_bounded_source_only_behavior(tmp_path, source, expression, expected): + _write(tmp_path, {"candidate.py": source + f"candidate = {expression}\n"}) + resolver = ImportResolver(tmp_path) + module = resolver.module(tmp_path / "candidate.py") + members = tool_list_candidates(module.tree.body[-1].value, module, resolver) + assert [(ast.unparse(member.expr), member.conditions) for member in members] == expected diff --git a/tests/test_imported_tool_review.py b/tests/test_imported_tool_review.py index e7e148ef..72c7c4a3 100644 --- a/tests/test_imported_tool_review.py +++ b/tests/test_imported_tool_review.py @@ -2501,10 +2501,15 @@ def test_the_real_readers_still_read(repo, reader, established): assert (result["comparison_status"] == "compared") is established, result["head"]["limits"] assert _rows(result) == [("agent", "execute", "added")] else: - # Library readers need an explicit retained-namespace proof; a named - # mutable list cannot enumerate its members under that uncertainty. + # Library readers still lack retained-namespace proof. Readable + # initializer members are candidates, never established bindings. assert result["comparison_status"] == "partial" - assert result["rows"] == [] + assert _rows(result) == [ + ("agent", "execute", "not_established"), + ("agent", "lookup", "not_established"), + ] + assert all(row["uncertainty"] for row in result["rows"]) + assert next(row for row in result["rows"] if row["tool"] == "execute")["candidate_change"] == "added" assert any("dynamic tools expression" in gap["reason"] for gap in result["head"]["coverage_gaps"]) assert any("constructor identity is not established" in gap["reason"] for gap in result["head"]["coverage_gaps"])