From 710ccc41f5628f8aaf971cd66f7ad0d0ad78322f Mon Sep 17 00:00:00 2001 From: bouillipx Date: Tue, 3 Mar 2026 11:04:32 +0800 Subject: [PATCH 1/3] test(agent): close A-101 with direct internal branch coverage Finish the remaining verification task for refactor-dare-agent-structure-split by adding direct unit coverage for the extracted internal execution modules.\n\nKey changes:\n- add targeted direct unit tests in tests/unit/test_dare_agent_orchestration_split.py for execute hook-policy block, execute no-tool success, tool preflight deny, tool done-predicate retry, and milestone plan-policy failure\n- mark OpenSpec task 3.1 complete with fresh command/result evidence in openspec/changes/refactor-dare-agent-structure-split/tasks.md\n- refresh docs/design/modules/agent/TODO.md evidence wording and updated date to reflect the new internal branch coverage\n- add docs/features/refactor-dare-agent-structure-split.md as the change status/evidence source and link the historical implementation PR\n\nRationale:\nThe A-101 refactor already moved DareAgent orchestration into _internal modules, but the remaining task still required branch-level tests that exercise those modules directly rather than only asserting facade delegation. This commit closes that gap without changing runtime behavior, so the change can move to review and archive once merged. --- docs/design/modules/agent/TODO.md | 4 +- .../refactor-dare-agent-structure-split.md | 60 +++ .../tasks.md | 5 +- .../test_dare_agent_orchestration_split.py | 391 +++++++++++++++++- 4 files changed, 456 insertions(+), 4 deletions(-) create mode 100644 docs/features/refactor-dare-agent-structure-split.md diff --git a/docs/design/modules/agent/TODO.md b/docs/design/modules/agent/TODO.md index e1ca28c4..9f2c7f5e 100644 --- a/docs/design/modules/agent/TODO.md +++ b/docs/design/modules/agent/TODO.md @@ -1,6 +1,6 @@ # Agent TODO -> 更新日期：2026-02-27 +> 更新日期：2026-03-03 > 说明：Agent 模块唯一补齐清单（用于范围讨论与执行跟踪） ## 1. 本轮范围建议（用于你我讨论） @@ -34,7 +34,7 @@ - `dare_framework/agent/_internal/milestone_orchestrator.py` - `dare_framework/agent/_internal/execute_engine.py` - `dare_framework/agent/_internal/tool_executor.py` - - `tests/unit/test_dare_agent_orchestration_split.py`（新增委托边界测试） + - `tests/unit/test_dare_agent_orchestration_split.py`（委托边界测试 + `_internal` execute/tool/milestone 分支定向单测） - 受影响回归：`tests/unit/test_five_layer_agent.py`、`tests/unit/test_dare_agent_hook_governance.py`、`tests/unit/test_dare_agent_hook_transport_boundary.py` ### A-102 step-driven 路径闭环（P1） diff --git a/docs/features/refactor-dare-agent-structure-split.md b/docs/features/refactor-dare-agent-structure-split.md new file mode 100644 index 00000000..5a6eef8d --- /dev/null +++ b/docs/features/refactor-dare-agent-structure-split.md @@ -0,0 +1,60 @@ +--- +change_ids: ["refactor-dare-agent-structure-split"] +doc_kind: feature +topics: ["agent", "refactor", "orchestration", "testing"] +created: 2026-03-03 +updated: 2026-03-03 +status: active +mode: openspec +--- + +# Feature: refactor-dare-agent-structure-split + +## Scope + +完成 A-101 的最后一项收尾：在 `DareAgent` façade delegation 已拆分完成的前提下，为 `_internal` 的 `execute_engine`、`tool_executor`、`milestone_orchestrator` 补齐 direct unit tests，锁住 success/failure/approval/policy/retry 分支语义。 + +## OpenSpec Artifacts + +- Proposal: `openspec/changes/refactor-dare-agent-structure-split/proposal.md` +- Design: `openspec/changes/refactor-dare-agent-structure-split/design.md` +- Tasks: `openspec/changes/refactor-dare-agent-structure-split/tasks.md` + +## Governance Anchors + +- `docs/design/modules/agent/TODO.md` +- `docs/design/TODO_INDEX.md` + +## Evidence + +### Commands + +- `git worktree add .worktrees/refactor-dare-agent-structure-split -b codex/refactor-dare-agent-structure-split origin/main` +- `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py tests/unit/test_five_layer_agent.py tests/unit/test_dare_agent_hook_governance.py tests/unit/test_dare_agent_hook_transport_boundary.py` +- `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py -k 'before_model_hook_blocks or no_tool_calls or preflight_denies or done_predicate_is_satisfied or plan_policy_failure'` +- `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py` +- `../../.venv/bin/python -m pytest -q tests/unit/test_five_layer_agent.py tests/unit/test_dare_agent_hook_governance.py tests/unit/test_dare_agent_hook_transport_boundary.py` + +### Results + +- `git worktree add ... origin/main`: created an isolated continuation workspace for the final A-101 closeout from `origin/main` commit `5d1cfb4`. +- `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py tests/unit/test_five_layer_agent.py tests/unit/test_dare_agent_hook_governance.py tests/unit/test_dare_agent_hook_transport_boundary.py`: baseline passed (`43 passed, 1 warning`) before adding new tests, confirming the change started from a clean regression surface. +- `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py -k 'before_model_hook_blocks or no_tool_calls or preflight_denies or done_predicate_is_satisfied or plan_policy_failure'`: passed (`4 passed, 5 deselected, 1 warning`) after adding the new direct `_internal` branch coverage. +- `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py`: passed (`9 passed, 1 warning`) with both the existing façade delegation assertions and the new direct execution-unit tests. +- `../../.venv/bin/python -m pytest -q tests/unit/test_five_layer_agent.py tests/unit/test_dare_agent_hook_governance.py tests/unit/test_dare_agent_hook_transport_boundary.py`: passed (`39 passed, 1 warning`) after the new unit tests landed, confirming no regression on the previously accepted A-101 coverage surface. + +### Behavior Verification + +- Happy path: `_internal` execute logic now has a direct test proving model responses without tool calls finalize successfully and persist the assistant message into STM without going back through the `DareAgent` façade. +- Happy path: `_internal` tool execution now has a direct retry test proving `done_predicate` retries until the required output key is produced, while preserving the existing success result shape. +- Error branch: `_internal` execute/model hook block, tool preflight deny, and milestone plan-policy failure now each have direct tests asserting their structured failure payloads/events rather than relying on broader integration suites to catch drift. + +### Risks and Rollback + +- Risk: these tests use lightweight fake agents rather than full `DareAgent` instances, so future internal protocol expansion may require updating the fakes alongside implementation changes. +- Rollback: revert the new direct unit tests and tasks/docs evidence updates; production runtime behavior is unchanged by this slice. + +### Review and Merge Gate Links + +- Historical implementation PR (merged): `https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/pull/117` +- Historical owner feedback thread: `https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/pull/117` diff --git a/openspec/changes/refactor-dare-agent-structure-split/tasks.md b/openspec/changes/refactor-dare-agent-structure-split/tasks.md index 4942bd2e..7cfe7853 100644 --- a/openspec/changes/refactor-dare-agent-structure-split/tasks.md +++ b/openspec/changes/refactor-dare-agent-structure-split/tasks.md @@ -13,7 +13,10 @@ ## 3. Verification and regression protection -- [ ] 3.1 Add/adjust targeted unit tests for extracted execution units (success/failure/approval/policy/retry branches). +- [x] 3.1 Add/adjust targeted unit tests for extracted execution units (success/failure/approval/policy/retry branches). + Evidence: `tests/unit/test_dare_agent_orchestration_split.py`（新增 execute hook-policy block、execute no-tool success、tool preflight deny、tool done-predicate retry、milestone plan-policy failure 五条 direct unit tests，直接覆盖 `_internal` 模块而非仅 facade delegation） + Commands: `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py -k 'before_model_hook_blocks or no_tool_calls or preflight_denies or done_predicate_is_satisfied or plan_policy_failure'` => `4 passed, 5 deselected, 1 warning`；`../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py` => `9 passed, 1 warning`；`../../.venv/bin/python -m pytest -q tests/unit/test_five_layer_agent.py tests/unit/test_dare_agent_hook_governance.py tests/unit/test_dare_agent_hook_transport_boundary.py` => `39 passed, 1 warning` + Last Updated: `2026-03-03` - [x] 3.2 Run affected unit test suites for agent execute/tool/milestone paths and fix regressions. - [x] 3.3 Run design-doc drift and compile checks to ensure governance and structural integrity remain green. diff --git a/tests/unit/test_dare_agent_orchestration_split.py b/tests/unit/test_dare_agent_orchestration_split.py index ffd99517..6e8c0b68 100644 --- a/tests/unit/test_dare_agent_orchestration_split.py +++ b/tests/unit/test_dare_agent_orchestration_split.py @@ -1,15 +1,22 @@ from __future__ import annotations +from types import SimpleNamespace from typing import Any import pytest +from dare_framework.agent._internal.execute_engine import run_execute_loop +from dare_framework.agent._internal.milestone_orchestrator import run_milestone_loop from dare_framework.agent._internal.orchestration import MilestoneResult, SessionState +from dare_framework.agent._internal.tool_executor import run_tool_loop +from dare_framework.agent.dare_agent import SecurityPreflightResult from dare_framework.agent.dare_agent import DareAgent from dare_framework.config import Config from dare_framework.context import Context +from dare_framework.hook.types import HookDecision, HookPhase, HookResult from dare_framework.model.types import ModelInput, ModelResponse -from dare_framework.plan.types import Milestone, RunResult, Task, ToolLoopRequest +from dare_framework.plan.types import DonePredicate, Envelope, Milestone, RunResult, Task, ToolLoopRequest, VerifyResult +from dare_framework.security import PolicyDecision, RiskLevel, TrustedInput from dare_framework.tool.types import ToolResult @@ -39,6 +46,292 @@ def _build_agent() -> DareAgent: ) +class _RecordingBudgetContext: + def __init__(self, *, assembled: Any | None = None) -> None: + self.assembled = assembled or SimpleNamespace(messages=[], tools=[], metadata={}) + self.budget_checks = 0 + self.budget_uses: list[tuple[str, int]] = [] + self.stm_messages: list[Any] = [] + + def budget_check(self) -> None: + self.budget_checks += 1 + + def budget_use(self, bucket: str, amount: int) -> None: + self.budget_uses.append((bucket, amount)) + + def assemble(self) -> Any: + return self.assembled + + def stm_add(self, message: Any) -> None: + self.stm_messages.append(message) + + +class _RecordingModelAdapter: + def __init__(self, response: ModelResponse) -> None: + self.name = "internal-model" + self.calls = 0 + self.response = response + + async def generate(self, model_input: ModelInput, *, options: Any = None) -> ModelResponse: + _ = (model_input, options) + self.calls += 1 + return self.response + + +class _InternalExecuteAgent: + def __init__( + self, + *, + model_response: ModelResponse, + hook_results: dict[HookPhase, HookResult] | None = None, + ) -> None: + self._context = _RecordingBudgetContext() + self._execution_mode = "model_driven" + self._exec_ctl = None + self._max_tool_iterations = 2 + self._model = _RecordingModelAdapter(model_response) + self._hook_results = hook_results or {} + self.hook_calls: list[tuple[HookPhase, dict[str, Any]]] = [] + self.logged_events: list[tuple[str, dict[str, Any]]] = [] + self.finalized_results: list[dict[str, Any]] = [] + + async def _emit_hook(self, phase: HookPhase, payload: dict[str, Any]) -> HookResult: + self.hook_calls.append((phase, dict(payload))) + return self._hook_results.get(phase, HookResult(decision=HookDecision.ALLOW)) + + async def _log_event(self, event_type: str, payload: dict[str, Any]) -> None: + self.logged_events.append((event_type, dict(payload))) + + async def _capability_index(self) -> dict[str, Any]: + return {} + + async def _run_tool_loop( + self, + request: Any, + *, + transport: Any | None, + tool_name: str, + tool_call_id: str, + descriptor: Any | None = None, + ) -> dict[str, Any]: + raise AssertionError(f"tool loop should not run in this execute test: {request}, {transport}, {tool_name}, {tool_call_id}, {descriptor}") + + async def _run_step_driven_execute_loop( + self, + plan: Any, + execute_start: float, + *, + transport: Any | None = None, + ) -> dict[str, Any]: + raise AssertionError(f"step-driven path should not run in this execute test: {plan}, {execute_start}, {transport}") + + async def _finalize_execute(self, start_time: float, result: dict[str, Any]) -> dict[str, Any]: + _ = start_time + self.finalized_results.append(dict(result)) + return result + + def _apply_context_patch(self, assembled: Any, dispatch: Any) -> tuple[list[Any], list[Any], dict[str, Any]]: + _ = dispatch + return list(assembled.messages), list(assembled.tools), dict(assembled.metadata) + + def _apply_model_input_patch(self, model_input: ModelInput, dispatch: Any) -> ModelInput: + _ = dispatch + return model_input + + def _context_stats(self, messages: list[Any], tools_count: int) -> dict[str, int]: + return {"messages_count": len(messages), "tools_count": tools_count} + + def _log(self, message: str) -> None: + _ = message + + def _log_model_messages(self, messages: list[Any], *, stage: str) -> None: + _ = (messages, stage) + + def _poll_or_raise(self) -> None: + return None + + def _record_token_usage(self, usage: dict[str, Any] | None) -> None: + _ = usage + + def _total_tokens_from_usage(self, usage: dict[str, Any]) -> int: + _ = usage + return 0 + + def _budget_stats(self) -> dict[str, Any]: + return {"budget_checks": self._context.budget_checks, "budget_uses": len(self._context.budget_uses)} + + def _is_plan_tool_call(self, name: str | None, descriptor: Any | None) -> bool: + _ = (name, descriptor) + return False + + def _is_skill_tool_call(self, descriptor: Any | None) -> bool: + _ = descriptor + return False + + def _mount_skill_from_result(self, output: Any) -> None: + raise AssertionError(f"skill mounting should not run in this execute test: {output}") + + +class _ToolGatewaySequence: + def __init__(self, results: list[ToolResult[dict[str, Any]]]) -> None: + self._results = list(results) + self.invoke_calls = 0 + + async def invoke(self, capability_id: str, approval_ctx: Any, *, envelope: Any, **params: Any) -> ToolResult[dict[str, Any]]: + _ = (capability_id, approval_ctx, envelope, params) + self.invoke_calls += 1 + return self._results[min(self.invoke_calls - 1, len(self._results) - 1)] + + +class _AllowExecuteSafeBoundary: + async def execute_safe(self, *, action: str, fn: Any, sandbox: Any) -> Any: + _ = (action, sandbox) + result = fn() + if hasattr(result, "__await__"): + return await result + return result + + +class _InternalToolAgent: + def __init__( + self, + *, + preflight: SecurityPreflightResult, + gateway_results: list[ToolResult[dict[str, Any]]] | None = None, + max_calls: int | None = None, + ) -> None: + milestone = Milestone(milestone_id="m1", description="tool test", user_input="tool test") + self._approval_manager = None + self._context = _RecordingBudgetContext() + self._governed_tool_gateway = _ToolGatewaySequence(gateway_results or [ToolResult(success=True, output={"ok": True})]) + self._security_boundary = _AllowExecuteSafeBoundary() + self._session_state = SimpleNamespace( + run_id="run-1", + current_milestone_state=SessionState(task_id="task-1", milestone_states=[]).current_milestone_state, + ) + self._preflight = preflight + self._max_calls = max_calls + self.logged_events: list[tuple[str, dict[str, Any]]] = [] + self.hook_calls: list[tuple[HookPhase, dict[str, Any]]] = [] + self.milestone_state = SessionState(task_id="task-1", milestone_states=[]).current_milestone_state + self._session_state = SimpleNamespace( + run_id="run-1", + current_milestone_state=SimpleNamespace(add_evidence=lambda evidence: None), + ) + + async def _emit_hook(self, phase: HookPhase, payload: dict[str, Any]) -> HookResult: + self.hook_calls.append((phase, dict(payload))) + return HookResult(decision=HookDecision.ALLOW) + + async def _evaluate_tool_security( + self, + *, + request: ToolLoopRequest, + descriptor: Any | None, + tool_name: str, + tool_call_id: str, + attempt: int, + requires_approval_override: bool | None = None, + trusted_risk_level_override: Any | None = None, + ) -> SecurityPreflightResult: + _ = (request, descriptor, tool_name, tool_call_id, attempt, requires_approval_override, trusted_risk_level_override) + return self._preflight + + async def _log_event(self, event_type: str, payload: dict[str, Any]) -> None: + self.logged_events.append((event_type, dict(payload))) + + def _budget_stats(self) -> dict[str, Any]: + return {"budget_checks": self._context.budget_checks, "budget_uses": len(self._context.budget_uses)} + + def _requires_approval(self, descriptor: Any | None) -> bool: + _ = descriptor + return False + + def _risk_level_from_trusted_input(self, trusted_input: TrustedInput) -> int: + mapping = { + RiskLevel.READ_ONLY: 1, + RiskLevel.IDEMPOTENT_WRITE: 2, + RiskLevel.NON_IDEMPOTENT_EFFECT: 3, + RiskLevel.COMPENSATABLE: 4, + } + return mapping[trusted_input.risk_level] + + def _risk_level_value(self, descriptor: Any | None) -> int: + _ = descriptor + return 1 + + def _risk_level_value_from_envelope(self, envelope: Any) -> int: + _ = envelope + return 1 + + def _tool_loop_max_calls(self, envelope: Any) -> int | None: + _ = envelope + return self._max_calls + + +class _RecordingSandbox: + def __init__(self) -> None: + self.created: list[str] = [] + self.rolled_back: list[str] = [] + self.committed: list[str] = [] + + def create_snapshot(self, context: Any) -> str: + _ = context + snapshot_id = f"snap-{len(self.created) + 1}" + self.created.append(snapshot_id) + return snapshot_id + + def rollback(self, context: Any, snapshot_id: str) -> None: + _ = context + self.rolled_back.append(snapshot_id) + + def commit(self, snapshot_id: str) -> None: + self.committed.append(snapshot_id) + + +class _InternalMilestoneAgent: + def __init__(self, milestone: Milestone) -> None: + self._context = _RecordingBudgetContext() + self._session_state = SessionState(task_id="task-1", milestone_states=[]) + self._session_state.milestone_states.append(SimpleNamespace(attempts=0, add_reflection=lambda text: None)) + self._max_milestone_attempts = 1 + self._sandbox = _RecordingSandbox() + self._remediator = None + self._milestone = milestone + self.logged_events: list[tuple[str, dict[str, Any]]] = [] + self.hook_calls: list[tuple[HookPhase, dict[str, Any]]] = [] + self.plan_loop_calls = 0 + + async def _emit_hook(self, phase: HookPhase, payload: dict[str, Any]) -> HookResult: + self.hook_calls.append((phase, dict(payload))) + return HookResult(decision=HookDecision.ALLOW) + + async def _log_event(self, event_type: str, payload: dict[str, Any]) -> None: + self.logged_events.append((event_type, dict(payload))) + + async def _run_plan_loop(self, milestone: Milestone) -> Any: + assert milestone is self._milestone + self.plan_loop_calls += 1 + return SimpleNamespace(success=True, steps=[]) + + async def _run_execute_loop(self, plan: Any, *, transport: Any | None = None) -> dict[str, Any]: + raise AssertionError(f"execute loop should not run when plan policy already failed: {plan}, {transport}") + + async def _verify_milestone(self, execute_result: dict[str, Any], validated_plan: Any | None = None) -> VerifyResult: + raise AssertionError(f"verify should not run when plan policy already failed: {execute_result}, {validated_plan}") + + async def _check_plan_policy(self, milestone: Milestone, validated_plan: Any | None) -> tuple[str | None, str]: + assert milestone is self._milestone + _ = validated_plan + return "execute plan denied by security policy", "deny" + + def _budget_stats(self) -> dict[str, Any]: + return {"budget_checks": self._context.budget_checks} + + def _log(self, message: str) -> None: + _ = message + + @pytest.mark.asyncio async def test_session_loop_delegates_to_internal_runner(monkeypatch: pytest.MonkeyPatch) -> None: agent = _build_agent() @@ -149,3 +442,99 @@ async def _fake_runner( assert calls[0][3] == "echo" assert calls[0][4] == "tc-1" assert calls[0][5] == {"d": 1} + + +@pytest.mark.asyncio +async def test_run_execute_loop_returns_policy_error_when_before_model_hook_blocks() -> None: + agent = _InternalExecuteAgent( + model_response=ModelResponse(content="unused", tool_calls=[]), + hook_results={ + HookPhase.BEFORE_MODEL: HookResult(decision=HookDecision.BLOCK), + }, + ) + + result = await run_execute_loop(agent, None) + + assert result["success"] is False + assert result["errors"] == ["model invocation denied by hook policy"] + assert agent._model.calls == 0 + assert any(phase is HookPhase.BEFORE_MODEL for phase, _ in agent.hook_calls) + + +@pytest.mark.asyncio +async def test_run_execute_loop_returns_content_when_model_has_no_tool_calls() -> None: + agent = _InternalExecuteAgent( + model_response=ModelResponse(content="final answer", tool_calls=[]), + ) + + result = await run_execute_loop(agent, None) + + assert result["success"] is True + assert result["outputs"] == [{"content": "final answer"}] + assert len(agent._context.stm_messages) == 1 + assert agent._context.stm_messages[0].role == "assistant" + assert agent._context.stm_messages[0].content == "final answer" + + +@pytest.mark.asyncio +async def test_run_tool_loop_returns_not_allow_when_preflight_denies() -> None: + agent = _InternalToolAgent( + preflight=SecurityPreflightResult( + trusted_input=TrustedInput(params={"text": "blocked"}, risk_level=RiskLevel.READ_ONLY), + decision=PolicyDecision.DENY, + reason="security policy denied capability 'tool.echo'", + ), + ) + request = ToolLoopRequest(capability_id="tool.echo", params={"text": "blocked"}) + + result = await run_tool_loop(agent, request, transport=None, tool_name="echo", tool_call_id="call-1") + + assert result["success"] is False + assert result["status"] == "not_allow" + assert result["error"] == "security policy denied capability 'tool.echo'" + assert any(event_type == "security.policy_denied" for event_type, _ in agent.logged_events) + + +@pytest.mark.asyncio +async def test_run_tool_loop_retries_until_done_predicate_is_satisfied() -> None: + agent = _InternalToolAgent( + preflight=SecurityPreflightResult( + trusted_input=TrustedInput(params={"text": "retry"}, risk_level=RiskLevel.READ_ONLY), + decision=PolicyDecision.ALLOW, + reason=None, + ), + gateway_results=[ + ToolResult(success=True, output={"stage": 1}), + ToolResult(success=True, output={"done": True}), + ], + max_calls=2, + ) + request = ToolLoopRequest( + capability_id="tool.echo", + params={"text": "retry"}, + envelope=Envelope(done_predicate=DonePredicate(required_keys=["done"])), + ) + + result = await run_tool_loop(agent, request, transport=None, tool_name="echo", tool_call_id="call-2") + + assert result["success"] is True + assert result["output"] == {"done": True} + assert agent._governed_tool_gateway.invoke_calls == 2 + + +@pytest.mark.asyncio +async def test_run_milestone_loop_rolls_back_and_records_policy_failure() -> None: + milestone = Milestone(milestone_id="m1", description="guarded milestone", user_input="guarded milestone") + agent = _InternalMilestoneAgent(milestone) + + result = await run_milestone_loop(agent, milestone) + + assert result == MilestoneResult( + success=False, + outputs=[], + errors=["execute plan denied by security policy"], + verify_result=VerifyResult(success=False, errors=["execute plan denied by security policy"]), + ) + assert agent._sandbox.rolled_back == ["snap-1"] + assert any(event_type == "security.plan.policy" for event_type, _ in agent.logged_events) + assert any(event_type == "milestone.failed" for event_type, _ in agent.logged_events) From 8068a1417df4b7a00f7b8c220d0598547b6f335f Mon Sep 17 00:00:00 2001 From: bouillipx Date: Tue, 3 Mar 2026 11:33:15 +0800 Subject: [PATCH 2/3] docs(feature): record A-101 verification PR evidence Add the active verification PR link to docs/features/refactor-dare-agent-structure-split.md so the change's review and merge-gate evidence stays complete.\n\nKey changes:\n- record PR #163 in the feature aggregation doc review links section\n- keep the active change's evidence trail aligned with the current review gate\n\nRationale:\nThe verification work is now under review in PR #163. Recording that link in the feature doc keeps the governance evidence chain current and lets the evidence-truth gate resolve the latest merge path for this change. --- docs/features/refactor-dare-agent-structure-split.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/features/refactor-dare-agent-structure-split.md b/docs/features/refactor-dare-agent-structure-split.md index 5a6eef8d..5ba0de31 100644 --- a/docs/features/refactor-dare-agent-structure-split.md +++ b/docs/features/refactor-dare-agent-structure-split.md @@ -58,3 +58,4 @@ mode: openspec - Historical implementation PR (merged): `https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/pull/117` - Historical owner feedback thread: `https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/pull/117` +- Final verification PR: `https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/pull/163` From 831bb9f26dedd3bd42a5bbfa3e0e38cd93c44d67 Mon Sep 17 00:00:00 2001 From: bouillipx Date: Tue, 3 Mar 2026 12:13:00 +0800 Subject: [PATCH 3/3] test(agent): tighten done-predicate retry coverage Address the PR #163 review on the retry-coverage test for refactor-dare-agent-structure-split.\n\nKey changes:\n- raise max_calls in test_run_tool_loop_retries_until_done_predicate_is_satisfied from 2 to 3\n- document in the test that max_calls must exceed the expected completion point so the assertion proves early exit on done_predicate satisfaction\n- update docs/features/refactor-dare-agent-structure-split.md with the focused re-run command/result and the specific review-thread link\n\nRationale:\nWith max_calls equal to the completion point, a buggy implementation that simply ran until the budget ceiling could still satisfy the previous assertions. Keeping max_calls above the expected completion point makes invoke_calls == 2 a real proof that the loop short-circuits when the done predicate becomes satisfied. --- docs/features/refactor-dare-agent-structure-split.md | 3 +++ tests/unit/test_dare_agent_orchestration_split.py | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/features/refactor-dare-agent-structure-split.md b/docs/features/refactor-dare-agent-structure-split.md index 5ba0de31..e42d83fd 100644 --- a/docs/features/refactor-dare-agent-structure-split.md +++ b/docs/features/refactor-dare-agent-structure-split.md @@ -32,6 +32,7 @@ mode: openspec - `git worktree add .worktrees/refactor-dare-agent-structure-split -b codex/refactor-dare-agent-structure-split origin/main` - `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py tests/unit/test_five_layer_agent.py tests/unit/test_dare_agent_hook_governance.py tests/unit/test_dare_agent_hook_transport_boundary.py` - `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py -k 'before_model_hook_blocks or no_tool_calls or preflight_denies or done_predicate_is_satisfied or plan_policy_failure'` +- `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py::test_run_tool_loop_retries_until_done_predicate_is_satisfied` - `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py` - `../../.venv/bin/python -m pytest -q tests/unit/test_five_layer_agent.py tests/unit/test_dare_agent_hook_governance.py tests/unit/test_dare_agent_hook_transport_boundary.py` @@ -40,6 +41,7 @@ mode: openspec - `git worktree add ... origin/main`: created an isolated continuation workspace for the final A-101 closeout from `origin/main` commit `5d1cfb4`. - `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py tests/unit/test_five_layer_agent.py tests/unit/test_dare_agent_hook_governance.py tests/unit/test_dare_agent_hook_transport_boundary.py`: baseline passed (`43 passed, 1 warning`) before adding new tests, confirming the change started from a clean regression surface. - `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py -k 'before_model_hook_blocks or no_tool_calls or preflight_denies or done_predicate_is_satisfied or plan_policy_failure'`: passed (`4 passed, 5 deselected, 1 warning`) after adding the new direct `_internal` branch coverage. +- `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py::test_run_tool_loop_retries_until_done_predicate_is_satisfied`: passed (`1 passed, 1 warning`) after the PR #163 review fix raised `max_calls` above the expected completion point, so the retry coverage now proves the loop exits on `done_predicate` satisfaction instead of coincidentally stopping at the budget ceiling. - `../../.venv/bin/python -m pytest -q tests/unit/test_dare_agent_orchestration_split.py`: passed (`9 passed, 1 warning`) with both the existing façade delegation assertions and the new direct execution-unit tests. - `../../.venv/bin/python -m pytest -q tests/unit/test_five_layer_agent.py tests/unit/test_dare_agent_hook_governance.py tests/unit/test_dare_agent_hook_transport_boundary.py`: passed (`39 passed, 1 warning`) after the new unit tests landed, confirming no regression on the previously accepted A-101 coverage surface. @@ -59,3 +61,4 @@ mode: openspec - Historical implementation PR (merged): `https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/pull/117` - Historical owner feedback thread: `https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/pull/117` - Final verification PR: `https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/pull/163` +- PR #163 review thread: `https://github.com/zts212653/Deterministic-Agent-Runtime-Engine/pull/163#discussion_r2875871921` diff --git a/tests/unit/test_dare_agent_orchestration_split.py b/tests/unit/test_dare_agent_orchestration_split.py index 6e8c0b68..1b470e1a 100644 --- a/tests/unit/test_dare_agent_orchestration_split.py +++ b/tests/unit/test_dare_agent_orchestration_split.py @@ -507,7 +507,9 @@ async def test_run_tool_loop_retries_until_done_predicate_is_satisfied() -> None ToolResult(success=True, output={"stage": 1}), ToolResult(success=True, output={"done": True}), ], - max_calls=2, + # Keep max_calls above the expected completion point so the assertion + # proves the loop exits on the done predicate rather than budget exhaustion. + max_calls=3, ) request = ToolLoopRequest( capability_id="tool.echo",