From 491db7bc5aa1d4c6ede8d19327f6b0fa3dd4bd95 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Sun, 11 Oct 2026 14:22:21 +0000 Subject: [PATCH] Seal attempt work for #182 (succeeded) Attempt-Id: 2026-10-11T14:14:59.170367Z --- simple_coding_agent/model_execution.py | 112 +++++++++-- tests/test_operator_handoff.py | 263 +++++++++++++++++++++++++ 2 files changed, 356 insertions(+), 19 deletions(-) diff --git a/simple_coding_agent/model_execution.py b/simple_coding_agent/model_execution.py index 1ab51d4..67087e7 100644 --- a/simple_coding_agent/model_execution.py +++ b/simple_coding_agent/model_execution.py @@ -348,7 +348,9 @@ def __init__( self._operator_handoff_reporter: OperatorHandoffReporter | None = None self._operator_handoff: OperatorHandoff | None = None self._operator_context_delivered = False + self._operator_delivery_via: str | None = None self._operator_fallback_used = False + self._operator_fallback_was_blocked = False self._operator_begun = False self._operator_deadline_expired = False self._cli_session_id: str | None = None @@ -570,6 +572,75 @@ def _operator_fallback_blocked(self) -> bool: return self._hard_token_limit_reached() + def _operator_delivery_label(self) -> str: + """How the handoff instruction reached the model, if it did. + + Only a main-thread tool boundary or the fallback follow-up counts as + delivered: a subagent boundary can never perform the handoff. + """ + + if not self._operator_context_delivered: + return "no" + if self._operator_delivery_via == "fallback": + return "fallback" + return "main-thread tool boundary" + + def _operator_fallback_label(self) -> str: + """Whether the single fallback follow-up was consumed, and how.""" + + if self._operator_fallback_was_blocked: + return "blocked" + if self._operator_fallback_used: + return "fired" + return "not due" + + def _operator_step_trail(self) -> str: + """One-line delivered/fallback/begun trail for expiry evidence.""" + + begun = "yes" if self._operator_begun else "no" + return ( + f"delivered={self._operator_delivery_label()}; " + f"fallback={self._operator_fallback_label()}; " + f"begun={begun}" + ) + + def _operator_expired_explanation(self) -> str: + """Explain an operator handoff expiry without misstating the steps. + + The old wording always claimed the model never invoked the handoff + skill, even when it had begun but not finished before the deadline. + """ + + latched = self._operator_handoff + request_id = latched.request_id if latched is not None else None + steps = self._operator_step_trail() + if self._operator_begun: + return ( + f"Operator handoff {request_id} reached its" + f" {_OPERATOR_HANDOFF_MODEL_DEADLINE_SECONDS}-second model" + " deadline after the model invoked the handoff skill but before" + f" it finished ({steps})." + ) + return ( + f"Operator handoff {request_id} reached its" + f" {_OPERATOR_HANDOFF_MODEL_DEADLINE_SECONDS}-second model" + " deadline without the model invoking the handoff skill" + f" ({steps})." + ) + + def _operator_expired_evidence( + self, observed_models: tuple[str, ...] + ) -> ModelExecution: + """Build the ``OPERATOR_HANDOFF_EXPIRED`` evidence for the latched request.""" + + return self._evidence( + ModelExecutionStatus.OPERATOR_HANDOFF_EXPIRED, + self._operator_expired_explanation(), + None, + None, + observed_models, + ) + async def _run_operator_fallback( self, client: SDKClient, observed_models: list[str] ) -> tuple[ResultMessage | None, tuple[str, ...]] | None: @@ -586,6 +657,7 @@ async def _run_operator_fallback( if latched is None: return None if self._operator_fallback_blocked(): + self._operator_fallback_was_blocked = True self._log( "operator_handoff_fallback_blocked", f"request={latched.request_id}; token budget reached the hard" @@ -594,6 +666,7 @@ async def _run_operator_fallback( ) return None self._operator_context_delivered = True + self._operator_delivery_via = "fallback" self._report_operator("delivered") self._log( "operator_handoff_fallback", @@ -719,7 +792,9 @@ async def execute( self._ensure_ledger_thread("main", prompt_chars=len(issue_body)) self._operator_handoff = None self._operator_context_delivered = False + self._operator_delivery_via = None self._operator_fallback_used = False + self._operator_fallback_was_blocked = False self._operator_begun = False self._operator_deadline_expired = False self._cli_session_id = None @@ -800,15 +875,7 @@ async def execute( if terminal is None: if self._operator_deadline_expired and self._operator_handoff is not None: - return self._evidence( - ModelExecutionStatus.OPERATOR_HANDOFF_EXPIRED, - f"Operator handoff {self._operator_handoff.request_id} reached" - f" its {_OPERATOR_HANDOFF_MODEL_DEADLINE_SECONDS}-second model" - " deadline without the model invoking the handoff skill.", - None, - None, - observed_models, - ) + return self._operator_expired_evidence(observed_models) return self._evidence( ModelExecutionStatus.INFRASTRUCTURE_ERROR, "Claude SDK stream ended without a terminal ResultMessage.", @@ -1054,7 +1121,8 @@ async def _receive_terminal( self._log( "operator_handoff_model_deadline_expired", f"request={self._operator_handoff.request_id if self._operator_handoff else None}; " - f"deadline_seconds={_OPERATOR_HANDOFF_MODEL_DEADLINE_SECONDS}", + f"deadline_seconds={_OPERATOR_HANDOFF_MODEL_DEADLINE_SECONDS}; " + f"{self._operator_step_trail()}", ) return None, tuple(observed_models) wait = self._operator_stream_wait() @@ -1917,15 +1985,21 @@ async def _record_post_tool_use( # instruction stays withheld until they are stopped. pass if self._operator_handoff is not None and not self._operator_context_delivered: - self._operator_context_delivered = True - self._report_operator("delivered") - self._log( - "operator_handoff_context_delivered", - f"request={self._operator_handoff.request_id}", - ) - contexts.append( - _operator_handoff_instruction(self._issue_number, self._attempt_id) - ) + # Only the main thread can perform the handoff, so a subagent + # boundary neither receives the instruction nor marks it delivered: + # the single fallback stays due until a main-thread boundary or + # the follow-up query reaches the model. + if _is_main_thread(hook_input): + self._operator_context_delivered = True + self._operator_delivery_via = "post_tool" + self._report_operator("delivered") + self._log( + "operator_handoff_context_delivered", + f"request={self._operator_handoff.request_id}", + ) + contexts.append( + _operator_handoff_instruction(self._issue_number, self._attempt_id) + ) if contexts: return { "hookSpecificOutput": { diff --git a/tests/test_operator_handoff.py b/tests/test_operator_handoff.py index 87490e9..126b1ae 100644 --- a/tests/test_operator_handoff.py +++ b/tests/test_operator_handoff.py @@ -507,3 +507,266 @@ def bad_reporter(event: str, request_id: str) -> None: ) assert "operator_request" in reply["hookSpecificOutput"]["additionalContext"] + + +# --- regression tests: subagent boundaries must not consume delivery (issue #182) + + +def subagent_hook_input(*, agent_id: str = "sub-1", tool_name: str = "Edit") -> dict: + return { + "tool_name": tool_name, + "tool_input": {"file_path": "src/a.py"}, + "agent_id": agent_id, + } + + +def test_subagent_boundary_does_not_deliver_the_operator_instruction( + tmp_path: Path, +) -> None: + client = FakeClient(object(), [result()]) + executor, reported = make_executor( + tmp_path, client, handoff=OperatorHandoff("req-1", datetime.now(UTC)) + ) + + reply = asyncio.run(executor._record_post_tool_use(subagent_hook_input(), None, {})) + + assert reply == {} + assert executor._operator_context_delivered is False + assert reported == [] + + +def test_fallback_stays_due_after_subagent_only_boundaries(tmp_path: Path) -> None: + now = datetime.now(UTC) + client = FakeClient(object(), [result()]) + executor, _ = make_executor( + tmp_path, + client, + handoff=OperatorHandoff("req-1", now - timedelta(seconds=61)), + now=now, + ) + + asyncio.run(executor._record_post_tool_use(subagent_hook_input(), None, {})) + + assert executor._operator_context_delivered is False + assert executor._operator_due_action() == "fallback" + + +def test_main_thread_boundary_after_a_subagent_boundary_delivers_exactly_once( + tmp_path: Path, +) -> None: + client = FakeClient(object(), [result()]) + executor, reported = make_executor( + tmp_path, client, handoff=OperatorHandoff("req-1", datetime.now(UTC)) + ) + + subagent_reply = asyncio.run( + executor._record_post_tool_use(subagent_hook_input(), None, {}) + ) + first = asyncio.run( + executor._record_post_tool_use( + {"tool_name": "Edit", "tool_input": {"file_path": "src/a.py"}}, None, {} + ) + ) + second = asyncio.run( + executor._record_post_tool_use( + {"tool_name": "Edit", "tool_input": {"file_path": "src/b.py"}}, None, {} + ) + ) + + assert subagent_reply == {} + assert "operator_request" in first["hookSpecificOutput"]["additionalContext"] + assert second == {} + assert reported == [("delivered", "req-1")] + + +class HandoffInvokingClient(FakeClient): + """A fallback double whose follow-up turn invokes the handoff skill.""" + + def __init__(self, *args: object, executor_holder: list, **kwargs: object) -> None: + super().__init__(*args, **kwargs) + self.executor_holder = executor_holder + + async def query(self, prompt: str) -> None: + await super().query(prompt) + executor = self.executor_holder[0] + if executor is not None and "operator_request" in prompt: + await executor._record_skill_event( + "PreToolUse", skill_hook_input("handoff"), "tool-9", {} + ) + + +def test_fallback_after_subagent_only_boundaries_can_still_hand_off( + tmp_path: Path, +) -> None: + now = datetime.now(UTC) + holder: list = [None] + client = HandoffInvokingClient( + object(), + [result()], + followup_messages=[result()], + executor_holder=holder, + ) + executor, reported = make_executor( + tmp_path, + client, + handoff=OperatorHandoff("req-1", now - timedelta(seconds=61)), + now=now, + ) + holder[0] = executor + + async def run() -> object: + task = asyncio.ensure_future( + executor.execute(issue_body="Fix it.", working_directory=tmp_path) + ) + for _ in range(100): + await asyncio.sleep(0) + if client.queried_prompts: + break + # Only subagent boundaries happen while the request is pending. + await executor._record_post_tool_use(subagent_hook_input(), None, {}) + return await task + + execution = asyncio.run(run()) + + assert len(client.queried_prompts) == 1 + assert "operator_request" in client.queried_prompts[0] + assert execution.status is ModelExecutionStatus.HANDOFF_REQUESTED + assert reported == [("delivered", "req-1"), ("begun", "req-1")] + + +def test_expiry_after_begun_does_not_claim_the_skill_was_never_invoked( + tmp_path: Path, +) -> None: + now = [datetime.now(UTC)] + client = FakeClient(object(), [result()]) + events: list[tuple[str, str]] = [] + executor = ModelExecutor( + runtime_config(tmp_path), + client_factory=lambda options: client, + clock=lambda: now[0], + event_log=lambda event, + detail="", + level="INFO", + issue_number=None: events.append((event, detail)), + ) + executor.set_operator_handoff_provider(lambda: OperatorHandoff("req-1", now[0])) + + asyncio.run( + executor._record_post_tool_use( + {"tool_name": "Edit", "tool_input": {"file_path": "src/a.py"}}, None, {} + ) + ) + asyncio.run( + executor._record_skill_event( + "PreToolUse", skill_hook_input("handoff"), "tool-1", {} + ) + ) + now[0] = now[0] + timedelta(seconds=241) + terminal, models = asyncio.run(executor._receive_terminal(client)) + + assert terminal is None + expired = [ + detail + for event, detail in events + if event == "operator_handoff_model_deadline_expired" + ] + assert len(expired) == 1 + assert "req-1" in expired[0] + assert "begun=yes" in expired[0] + + execution = executor._operator_expired_evidence(models) + + assert execution.status is ModelExecutionStatus.OPERATOR_HANDOFF_EXPIRED + assert "req-1" in execution.explanation + assert "without the model invoking the handoff skill" not in execution.explanation + assert "begun=yes" in execution.explanation + + +def test_expiry_log_details_include_the_steps_reached(tmp_path: Path) -> None: + now = datetime.now(UTC) + client = FakeClient(object(), [result()]) + events: list[tuple[str, str]] = [] + executor = ModelExecutor( + runtime_config(tmp_path), + client_factory=lambda options: client, + clock=lambda: now, + event_log=lambda event, + detail="", + level="INFO", + issue_number=None: events.append((event, detail)), + ) + executor.set_operator_handoff_provider( + lambda: OperatorHandoff("req-1", now - timedelta(seconds=241)) + ) + + execution = asyncio.run( + executor.execute(issue_body="Fix it.", working_directory=tmp_path) + ) + + assert execution.status is ModelExecutionStatus.OPERATOR_HANDOFF_EXPIRED + expired = [ + detail + for event, detail in events + if event == "operator_handoff_model_deadline_expired" + ] + assert len(expired) == 1 + assert "delivered=no" in expired[0] + assert "fallback=" in expired[0] + assert "begun=no" in expired[0] + assert "delivered=no" in execution.explanation + assert "begun=no" in execution.explanation + + +class ClockAdvancingClient(FakeClient): + """A fallback double whose follow-up turn outlives the model deadline.""" + + def __init__(self, *args: object, advance=None, **kwargs: object) -> None: + super().__init__(*args, **kwargs) + self.advance = advance + + async def query(self, prompt: str) -> None: + await super().query(prompt) + if self.advance is not None: + self.advance() + + +def test_expiry_after_a_fired_fallback_reports_the_fallback_step( + tmp_path: Path, +) -> None: + now = [datetime.now(UTC)] + client = ClockAdvancingClient( + object(), + [result()], + followup_messages=[], + advance=lambda: now.__setitem__(0, now[0] + timedelta(seconds=200)), + ) + events: list[tuple[str, str]] = [] + executor = ModelExecutor( + runtime_config(tmp_path), + client_factory=lambda options: client, + clock=lambda: now[0], + event_log=lambda event, + detail="", + level="INFO", + issue_number=None: events.append((event, detail)), + ) + executor.set_operator_handoff_provider( + lambda: OperatorHandoff("req-1", now[0] - timedelta(seconds=61)) + ) + + execution = asyncio.run( + executor.execute(issue_body="Fix it.", working_directory=tmp_path) + ) + + assert execution.status is ModelExecutionStatus.OPERATOR_HANDOFF_EXPIRED + assert "delivered=fallback" in execution.explanation + assert "fallback=fired" in execution.explanation + assert "begun=no" in execution.explanation + expired = [ + detail + for event, detail in events + if event == "operator_handoff_model_deadline_expired" + ] + assert len(expired) == 1 + assert "delivered=fallback" in expired[0] + assert "fallback=fired" in expired[0]