test(agent): finish A-101 internal branch coverage for DareAgent split - #163
Merged
Conversation
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 710ccc41f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
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.
bouillipx
added a commit
that referenced
this pull request
Mar 3, 2026
Archive the completed refactor-dare-agent-structure-split OpenSpec change after PR #163 merged. Key changes: - run openspec archive so the completed change moves under openspec/changes/archive/2026-03-03-refactor-dare-agent-structure-split - sync the modularized DareAgent orchestration requirement back into openspec/specs/core-runtime/spec.md - move the feature aggregation record into docs/features/archive/ and rewrite artifact links to the archived change path - fix the archived tasks evidence note so task 3.1 no longer claims branch-level coverage is pending - record archive verification evidence for openspec list, full spec validation, and governance evidence truth Rationale: PR #163 completed the last implementation and review work for A-101, but the change still remained active in OpenSpec and the feature doc still lived under docs/features/. This commit performs the required completion-archive step so governance state, spec state, and evidence state all match the merged runtime history.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR closes the remaining verification gap in
refactor-dare-agent-structure-split._internalexecute/tool/milestone branches intests/unit/test_dare_agent_orchestration_split.py3.1complete with fresh evidence and refresh the agent design TODO evidence wordingdocs/features/refactor-dare-agent-structure-split.mdas the active feature aggregation doc for this changeWhy
PR #117 landed the A-101 structural refactor, but the change still had one explicit task open:
3.1 Add/adjust targeted unit tests for extracted execution units (success/failure/approval/policy/retry branches)The existing test file only verified facade delegation. This PR adds direct coverage for the extracted
_internalmodules so the change can move to archive without relying only on broader integration suites.Scope
Included:
_internalexecute/tool/milestone helpersrefactor-dare-agent-structure-splitNot included:
Validation
Executed:
../../.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.pyopenspec validate refactor-dare-agent-structure-split --type change --strict --json --no-interactive./scripts/ci/check_governance_evidence_truth.shResults:
_internaltests passed (4 passed, 5 deselected)9 passed)39 passed)1/1change valid)Review Focus
Please verify:
_internalbranches without overfitting implementation details