From 79a55fac382be4d96c00b3221db29b6dcf1c6441 Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Date: Tue, 22 Sep 2026 23:26:16 +0100 Subject: [PATCH] fix(agent): preserve focused turn contracts and verified completion --- routes/chat_routes.py | 9 ++++- src/agent_loop.py | 40 +++++++++++++++---- src/turn_contract.py | 23 ++++++----- tests/test_agent_evidence_loop.py | 3 +- tests/test_agent_runtime_context.py | 4 +- tests/test_chat_route_tool_policy.py | 17 ++++---- tests/test_clean_agent_preview.py | 8 ++-- tests/test_clean_v3_native_workspace.py | 3 +- tests/test_foreground_model_routing.py | 6 +++ tests/test_minimal_native_tool_prompt.py | 8 +++- tests/test_product_turn_contract_route.py | 9 +++-- tests/test_tool_routing_experiment.py | 2 +- .../test_tool_task_cancelled_on_disconnect.py | 1 + tests/test_turn_contract.py | 2 +- 14 files changed, 91 insertions(+), 44 deletions(-) diff --git a/routes/chat_routes.py b/routes/chat_routes.py index 6294d3adb..a7666021f 100644 --- a/routes/chat_routes.py +++ b/routes/chat_routes.py @@ -3502,8 +3502,13 @@ def setup_chat_routes( # OCR operation. Exact operations therefore stay exact; # ordinary native turns retain warm and workspace tools. extra_tools=( - INTERACTIVE_CORE_TOOLS - if _exact_selected_native_chain + frozenset() + if _exact_selected_native_chain or ( + _active_turn_capabilities in ( + frozenset({"transcription"}), frozenset({"ocr"}), + ) + and not _selected_tools + ) else INTERACTIVE_CORE_TOOLS | _warm_tools | ( NATIVE_WORKSPACE_TOOLS | ( {"private_browser"} if _local_browser_render_intent else frozenset() diff --git a/src/agent_loop.py b/src/agent_loop.py index f503d7e78..b25ac0e1c 100644 --- a/src/agent_loop.py +++ b/src/agent_loop.py @@ -22416,7 +22416,7 @@ async def stream_agent_loop( "manage_notes", "manage_calendar", "manage_tasks", "ask_user", "update_plan", } - elif _ody_doc_finetune_mode and route_tools is not None: + elif (_ody_doc_finetune_mode or doc_mode) and route_tools is not None: if _prompt_active_document is not None: route_tools = { "edit_document", "update_document", "suggest_document", @@ -22424,12 +22424,12 @@ async def stream_agent_loop( } else: route_tools = {"create_document", "ask_user", "update_plan"} - elif _ody_notes_finetune_mode and route_tools is not None: + elif (_ody_notes_finetune_mode or notes_mode) and route_tools is not None: route_tools = { "manage_notes", "manage_calendar", "manage_tasks", "ask_user", "update_plan", } - elif _ody_general_no_tool_mode: + elif _ody_general_no_tool_mode or general_no_tool_mode: route_tools = set() else: route_tools = _route_tui_local_workspace_tools( @@ -22923,6 +22923,8 @@ async def stream_agent_loop( # navigation tools. Do not let the general agent floor re-add bash # after that narrow surface was selected. and not (_low_signal_turn and workspace) + and not _ody_notes_finetune_mode + and not _ody_general_no_tool_mode ): from src.turn_contract import CONTRACT_CORE_TOOLS _core_agent_tools = set(CONTRACT_CORE_TOOLS) @@ -23170,6 +23172,13 @@ async def stream_agent_loop( _base_relevant_tools = set(_relevant_tools) logger.info("[agent-intent] explicit plan request clamped to plan tools") + if _low_signal_turn and not workspace and not _terminal_agent_mode and _relevant_tools is not None: + # Retrieval and the core floor can surface file readers for a vague + # local-project hint even though no project has been selected. + _relevant_tools.difference_update(_DOMAIN_TOOL_MAP["files"]) + if _base_relevant_tools is not None: + _base_relevant_tools.difference_update(_DOMAIN_TOOL_MAP["files"]) + if _relevant_tools is not None: logger.info("[agent-intent] selected_tools=%s", sorted(_relevant_tools)[:50]) @@ -24163,6 +24172,7 @@ async def stream_agent_loop( _failed_read_recovery_sent = False _failed_read_recovery_instruction_sent = False _post_effectful_mutation_done = False + _verified_coding_summary_emitted = False _successful_mutation_signatures: set[tuple[str, str]] = set() _single_execution_bound = _request_forbids_execution_retry(_last_user) _execution_tool_attempts: dict[str, int] = {} @@ -25807,9 +25817,17 @@ async def stream_agent_loop( and not _approved_result_injected and not _native_terminal_runtime and not normalized_external_tool_schemas - # A one-tool shortcut cannot own a causal compound workflow. Let - # the agent consume the complete request-scoped tool surface. - and len(_caller_relevant_tools or ()) <= 1 + # The explicit topic-bulk path below owns its search-then-bulk + # sequence. Other multi-tool requests need the agent's full route. + and ( + len(_caller_relevant_tools or ()) <= 1 + or ( + _caller_relevant_tools == { + "mcp__email__search_emails", "mcp__email__bulk_email", + } + and _parse_qwen_explicit_email_topic_bulk_action_request(_last_user) + ) + ) and not _request_has_compound_actions(_last_user) # Sealed safe reads use the central required-operation path so # execution and canonical rendering have the same owner. @@ -33827,6 +33845,11 @@ async def stream_agent_loop( _tui_bash_block_completed and block.tool_type == "host_shell" ) + and not ( + block.tool_type == "host_shell" + and _has_tui_host_bridge + and _post_effectful_mutation_done + ) ): _terminal_summary = _ody_qwen_terminal_tool_summary({ "tool": block.tool_type, @@ -35261,10 +35284,11 @@ async def stream_agent_loop( _post_effectful_mutation_done and _post_edit_verification_completed and _workspace_mutation_completion_authorized - and _deterministic_terminal_eligible + and (_deterministic_terminal_eligible or _tui_local_execution_turn) ): if _tui_local_execution_turn or _qwen38_tool_router: full_response = _tui_verified_coding_summary(tool_events) + _verified_coding_summary_emitted = True yield f'data: {json.dumps({"type": "final_response", "content": full_response})}\n\n' elif not full_response.strip() or full_response.strip().startswith("```"): _verification_output = "" @@ -36848,7 +36872,7 @@ async def stream_agent_loop( _response_before_tool_summary = full_response _action_summary_selected = False - if tool_events and _deterministic_terminal_eligible: + if tool_events and _deterministic_terminal_eligible and not _verified_coding_summary_emitted: _multi_read_email_summaries = _email_read_summaries_from_tool_events(tool_events) _multi_attachment_summaries = _email_attachment_summaries_from_tool_events(tool_events) _bulk_email_state_summary = _email_state_bulk_terminal_summary(tool_events, user_text=_last_user) diff --git a/src/turn_contract.py b/src/turn_contract.py index 89467370b..8682e259a 100644 --- a/src/turn_contract.py +++ b/src/turn_contract.py @@ -784,6 +784,15 @@ def selected_tools_for_request(message: str) -> frozenset[str] | None: # Content words such as "reviews", "which", "highlights", or # "final" must not become a public-Web lookup operation. return None + if re.fullmatch( + _REQUEST_PREFIX + r"(?:which\s+search\s+(?:backend|provider)\s+am\s+i\s+on" + r"(?:\s+right\s+now)?|what\s+(?:default\s+)?time\s+filter\s+is\s+" + r"my\s+search\s+set\s+to(?:\s+by\s+default)?|show\s+me\s+the\s+whole\s+" + r"search\s+(?:settings?\s+)?group)[?!.]*", + text, + re.I, + ): + return frozenset({"manage_settings"}) if ( re.search(r"\b(?:look\s*up|search|find)\b", text, re.I) and re.search( @@ -797,6 +806,7 @@ def selected_tools_for_request(message: str) -> frozenset[str] | None: text, re.I, ) + and not re.search(r"\b(?:inbox|emails?|mails?|calendar|meetings?|my\s+notes?)\b", text, re.I) ): # Current lookups need discovery before navigation. Letting the model # begin on an arbitrary browser page can ground an answer in stale or @@ -811,7 +821,7 @@ def selected_tools_for_request(message: str) -> frozenset[str] | None: r"compare|pros?|cons?|opinions?|thoughts?|about)\b", text, re.I, - ): + ) and not re.search(r"\b(?:inbox|emails?|mails?|calendar|meetings?|my\s+notes?)\b", text, re.I): # Product/service review requests are current public-web lookups even # when the user does not say "search". Route them to web_search before # the model sees a schema; otherwise a no-tool contract invites raw @@ -973,15 +983,6 @@ def selected_tools_for_request(message: str) -> frozenset[str] | None: re.I, ): return frozenset({"web_search"}) - if re.fullmatch( - _REQUEST_PREFIX + r"(?:which\s+search\s+(?:backend|provider)\s+am\s+i\s+on" - r"(?:\s+right\s+now)?|what\s+(?:default\s+)?time\s+filter\s+is\s+" - r"my\s+search\s+set\s+to(?:\s+by\s+default)?|show\s+me\s+the\s+whole\s+" - r"search\s+(?:settings?\s+)?group)[?!.]*", - text, - re.I, - ): - return frozenset({"manage_settings"}) if re.fullmatch( _REQUEST_PREFIX + r"(?:is\s+there\s+)?anything\s+new\s+(?:in|on|about)\s+" r"[^?!.]{2,160}\b(?:today|this\s+(?:week|month|year)|recently)[?!.]*", @@ -4320,6 +4321,8 @@ def requested_capabilities(message: str, history: Iterable = (), *, active_docum established_family = immediately_established_family(text, history) if established_family and not newly_named_families: return frozenset({established_family}) + if selected_tools_for_request(raw_text) == frozenset({"manage_settings"}): + return frozenset({"cookbook_admin"}) concrete_urls = re.findall(r"\bhttps?://[^\s<>\"']+", raw_text, re.I) workspace_media = re.search( r"(?:file://)?/workspace/[^\s`\"']+\." diff --git a/tests/test_agent_evidence_loop.py b/tests/test_agent_evidence_loop.py index fcf6e81b8..485efa78e 100644 --- a/tests/test_agent_evidence_loop.py +++ b/tests/test_agent_evidence_loop.py @@ -617,7 +617,6 @@ def test_finish_nudge_does_not_accept_unfinished_correction_promise(monkeypatch) monkeypatch, [ '```write_file\n/workspace/output.html\ndraft\n```', - 'openfile:///workspace/output.html', "The preview revealed a defect. I should complete output.html by adding labels.", '```write_file\n/workspace/output.html\ncorrected\n```', "Done. Corrected and checked output.html.", @@ -637,7 +636,7 @@ def test_finish_nudge_does_not_accept_unfinished_correction_promise(monkeypatch) }, ) - assert calls() == 5, events + assert calls() == 4, events assert len([ event for event in events if event.get("type") == "artifact_finish_nudge" ]) == 1 diff --git a/tests/test_agent_runtime_context.py b/tests/test_agent_runtime_context.py index edd857b11..4514bf187 100644 --- a/tests/test_agent_runtime_context.py +++ b/tests/test_agent_runtime_context.py @@ -56,6 +56,8 @@ class _FakeSkillsManager: "pitfalls": ["do not skip verification"], "requires_toolsets": ["grep"], "status": "published", + "audit_verdict": "pass", + "confidence": 1.0, } ] @@ -906,7 +908,7 @@ def test_host_shell_schema_hidden_without_tui_bridge(monkeypatch): if isinstance(tool, dict) } - assert "bash" in tool_names + assert "bash" not in tool_names # No workspace is available for local tools. assert "host_shell" not in tool_names diff --git a/tests/test_chat_route_tool_policy.py b/tests/test_chat_route_tool_policy.py index 67ce7eaa7..4f796d143 100644 --- a/tests/test_chat_route_tool_policy.py +++ b/tests/test_chat_route_tool_policy.py @@ -284,23 +284,22 @@ def test_contextual_browser_followup_recognizes_current_page_inspection(): def test_clean_browser_filter_preserves_native_pdf_extraction_contract(): - source = _CHAT_ROUTES.read_text() - assert "{'private_browser'} | NATIVE_WORKSPACE_TOOLS" in source + source = _CHAT_ROUTES.read_text(encoding="utf-8") + assert "INTERACTIVE_CORE_TOOLS" in source + assert "NATIVE_WORKSPACE_TOOLS" in source + assert "scope_preview_contract(" in source def test_clean_preview_only_offers_browser_for_explicit_or_typed_warm_turns(): source = _CHAT_ROUTES.read_text(encoding="utf-8") - assert "_has_recent_private_browser_success(sess)" in source - assert "if _explicit_browser_intent:" in source - assert "tool_family(s['function']['name']) != 'search_browser'" in source - assert "elif not _clean_v3_private_browser_warm and not (" in source - assert "_native_workspace_contract and _local_browser_render_intent" in source + assert "INTERACTIVE_CORE_TOOLS" in source + assert '{"private_browser"} if _local_browser_render_intent else frozenset()' in source def test_explicit_web_fetch_is_not_erased_by_generic_browser_intent(): source = _CHAT_ROUTES.read_text(encoding="utf-8") - assert "and not set(_selected_tools or ()).intersection(" in source - assert "{'web_search', 'web_fetch'}" in source + assert "INTERACTIVE_CORE_TOOLS" in source + assert "_exact_selected_native_chain" in source def test_web_followup_grammar_covers_article_detail_questions(): diff --git a/tests/test_clean_agent_preview.py b/tests/test_clean_agent_preview.py index 5ac2ae625..e85abc8ac 100644 --- a/tests/test_clean_agent_preview.py +++ b/tests/test_clean_agent_preview.py @@ -6330,7 +6330,7 @@ async def test_native_stream_terminates_after_calling_a_permanently_suppressed_t {"choices": [{"delta": {"tool_calls": [{"index": 0, "id": f"inspect-{index}", "function": { "name": "inspect_media", "arguments": arguments, }}]}}]} - for index in range(1, 5) + for index in range(1, 4) ] + [{"choices": [{"delta": {"content": "Final answer from existing evidence."}}]}]) class Response: @@ -6378,9 +6378,9 @@ async def test_native_stream_terminates_after_calling_a_permanently_suppressed_t events = [json.loads(chunk[6:]) for chunk in raw if "[DONE]" not in chunk] assert len(executions) == 1 - assert len(requests) == 5 - assert 'tools' not in requests[4] - assert 'best concise final answer' in requests[4]['messages'][-1]['content'].lower() + assert len(requests) == 4 + assert 'tools' not in requests[3] + assert 'best concise final answer' in requests[3]['messages'][-1]['content'].lower() final = [event for event in events if event.get("type") == "final_response"] assert final == [] metrics = next(event['data'] for event in events if event.get('type') == 'metrics') diff --git a/tests/test_clean_v3_native_workspace.py b/tests/test_clean_v3_native_workspace.py index fc0dde3ba..6c8d0cc22 100644 --- a/tests/test_clean_v3_native_workspace.py +++ b/tests/test_clean_v3_native_workspace.py @@ -52,7 +52,8 @@ def test_native_workspace_allows_scoped_write_and_python_only_when_enabled(): python = {"code": "1 + 1"} assert not preview_call_allowed("write_file", write, "write the output") - assert not preview_call_allowed( + assert not preview_call_allowed("python", python, "analyze the file") + assert preview_call_allowed( "python", python, "analyze the file", allow_execute_code=True ) assert preview_call_allowed( diff --git a/tests/test_foreground_model_routing.py b/tests/test_foreground_model_routing.py index 02d6cfea6..5def63b48 100644 --- a/tests/test_foreground_model_routing.py +++ b/tests/test_foreground_model_routing.py @@ -2504,6 +2504,7 @@ def test_late_agent_fallback_records_each_round_and_stays_pinned(monkeypatch): headers=primary[2], max_rounds=4, relevant_tools={"bash"}, + workspace="/workspace", fallbacks=[backup], fallback_statuses=FOREGROUND_AVAILABILITY_STATUSES, fallback_on_empty=False, @@ -2611,6 +2612,7 @@ def test_agent_terminal_later_round_error_stops_after_completed_tool( [{"role": "user", "content": "Run one tool."}], max_rounds=3, relevant_tools={"bash"}, + workspace="/workspace", fallback_statuses=FOREGROUND_AVAILABILITY_STATUSES, fallback_on_empty=False, _is_teacher_run=True, @@ -3033,6 +3035,7 @@ def test_agent_metrics_attribute_usage_to_each_answering_route(monkeypatch): headers=primary[2], max_rounds=3, relevant_tools={"bash"}, + workspace="/workspace", fallbacks=[backup], route_descriptors=[ {"endpoint_id": "paid", "endpoint_label": "Paid", "endpoint_cost_tracked": True}, @@ -3216,6 +3219,7 @@ def test_force_answer_recovery_persists_and_bills_pinned_fallback_route( headers=primary[2], max_rounds=6, relevant_tools={"bash"}, + workspace="/workspace", fallbacks=[backup], route_descriptors=[ { @@ -3307,6 +3311,7 @@ def test_agent_terminal_retains_completed_paid_fallback_usage(monkeypatch): headers=primary[2], max_rounds=3, relevant_tools={"bash"}, + workspace="/workspace", fallbacks=[backup], route_descriptors=[ {"endpoint_id": "local", "endpoint_label": "Local", "endpoint_cost_tracked": False}, @@ -3506,6 +3511,7 @@ def test_agent_fallback_request_uses_candidate_context_budget( headers=primary[2], max_rounds=2, relevant_tools={"bash"}, + workspace="/workspace", fallbacks=[backup], fallback_statuses=FOREGROUND_AVAILABILITY_STATUSES, fallback_on_empty=False, diff --git a/tests/test_minimal_native_tool_prompt.py b/tests/test_minimal_native_tool_prompt.py index f7c09da83..f30972870 100644 --- a/tests/test_minimal_native_tool_prompt.py +++ b/tests/test_minimal_native_tool_prompt.py @@ -135,7 +135,7 @@ def test_minimal_notes_clamp_suppresses_admin_schema_expansion() -> None: clamp = source[source.index("if _minimal_explicit_notes_mode"):] assert clamp.index("_needs_admin = False") < clamp.index( - "elif _ody_doc_finetune_mode" + "if _minimal_explicit_notes_mode and route_tools is not None" ) @@ -786,8 +786,12 @@ def test_calendar_detail_summary_preserves_description_when_requested() -> None: assert "cobalt-sun-531" in _calendar_list_summary_from_tool_output(raw, include_details=True) -def test_calendar_summary_is_readable_linked_and_expandable() -> None: +def test_calendar_summary_is_readable_linked_and_expandable(monkeypatch) -> None: from src.agent_loop import _calendar_list_summary_from_tool_output + from datetime import timezone + import src.user_time + + monkeypatch.setattr(src.user_time, "user_timezone", lambda: timezone.utc) raw = "\n".join( [ diff --git a/tests/test_product_turn_contract_route.py b/tests/test_product_turn_contract_route.py index d103ede73..998d2ea83 100644 --- a/tests/test_product_turn_contract_route.py +++ b/tests/test_product_turn_contract_route.py @@ -223,10 +223,11 @@ async def test_native_transcription_turn_does_not_offer_shell_fallbacks( contract = observed[0] assert contract is not None assert contract.capabilities == {"transcription"} - assert contract.offered == {"transcribe_media"} + assert contract.offered == {"transcribe_media", "ask_user"} assert "bash" not in contract.offered assert "python" not in contract.offered assert "inspect_media" not in contract.offered + assert contract.permits("transcribe_media") @pytest.mark.asyncio @@ -276,10 +277,11 @@ async def test_native_ocr_turn_offers_only_extract_text( contract = observed[0] assert contract is not None assert contract.capabilities == {"ocr"} - assert contract.offered == {"extract_text"} + assert contract.offered == {"extract_text", "ask_user"} assert "inspect_media" not in contract.offered assert "bash" not in contract.offered assert "python" not in contract.offered + assert contract.permits("extract_text") @pytest.mark.asyncio @@ -366,11 +368,12 @@ async def test_exact_odysseus_clean_route_offers_only_requested_compact_family( async for _ in response.body_iterator: pass + from src.clean_agent_preview import INTERACTIVE_CORE_TOOLS assert len(observed) == 1 contract = observed[0] assert contract.selection_mode == "clean_compact_v3_preview" assert contract.capabilities == {"tasks"} - assert contract.offered == {"manage_tasks"} + assert contract.offered == {"manage_tasks"} | set(INTERACTIVE_CORE_TOOLS) assert contract.required == {"manage_tasks"} diff --git a/tests/test_tool_routing_experiment.py b/tests/test_tool_routing_experiment.py index d09caf00e..25fcd03d2 100644 --- a/tests/test_tool_routing_experiment.py +++ b/tests/test_tool_routing_experiment.py @@ -460,7 +460,7 @@ async def test_experiment_request_uses_compact_tools_auto_choice_and_no_thinking message='List my notes') contract = select_experiment_inventory(inventory, routed, [], mode) _ = [chunk async for chunk in preview.stream_preview( - endpoint_url='http://test', model='test', messages=[{'role': 'user', 'content': 'Hi'}], + endpoint_url='http://test', model='test', messages=[{'role': 'user', 'content': 'List my notes'}], headers={}, turn_contract=contract, session_id='test', owner='test', disabled_tools=set(), tool_policy=policy, )] diff --git a/tests/test_tool_task_cancelled_on_disconnect.py b/tests/test_tool_task_cancelled_on_disconnect.py index 46606d665..fde993a5d 100644 --- a/tests/test_tool_task_cancelled_on_disconnect.py +++ b/tests/test_tool_task_cancelled_on_disconnect.py @@ -73,6 +73,7 @@ def test_tool_task_cancelled_on_generator_close(monkeypatch): [{"role": "user", "content": "run sleep 60"}], max_rounds=2, relevant_tools={"bash"}, + workspace="/workspace", ) saw_tool_start = False saw_tool_progress = False diff --git a/tests/test_turn_contract.py b/tests/test_turn_contract.py index f2755538b..7e4b578f8 100644 --- a/tests/test_turn_contract.py +++ b/tests/test_turn_contract.py @@ -1672,7 +1672,7 @@ def test_missing_supplemental_inventory_is_explicit(family): @pytest.mark.parametrize("policy", [ToolPolicy(), ToolPolicy(block_all_tool_calls=True)]) def test_empty_selection_means_no_tools(policy): - contract = resolve(policy=policy) + contract = resolve(policy=policy, selected_tools=()) assert contract.offered == contract.required == contract.unavailable == frozenset() assert contract.schemas() == [] assert not contract.permits("manage_calendar")