feat(agent-mesh): route require_approval through the action-bound coordinator (#3067)

[Carlos Hernandez (carloshvp)]

Summary

Step 2 of the ADR-0030 rollout (issue #3067, follows the merged foundation #3015): route require_approval policy decisions through the action-bound ApprovalCoordinator in govern().

Until now require_approval went straight to the legacy approval.py handler (synchronous approve/deny, no action binding, no audit linkage, post-hoc timeout). This PR wires it through the merged ADR-0030 coordinator while keeping the legacy path as the default.

How it works

When both approval_coordinator and approval_chain_id are set on govern() / GovernanceConfig, a require_approval decision:

1. is bound to the exact action via an ActionBinding (operation, agent, tool, parameters, subject) and its SHA-256 / JCS digest;
2. opens an approval request against the configured chain (carrying policy version and chain version);
3. uses the existing ApprovalHandler as the synchronous source of the approver vote, recorded as one authenticated stage-0 chain entry;
4. is revalidated immediately before execution (validate_for_execution): the action digest, policy version, and chain version must all still match, and the approval is consumed exactly once.

Anything short of a terminal allow denies, fail-closed:

• an approver identity the chain does not permit,
• an expired request (TTL via approval_ttl_seconds),
• a rejection.

Audit entries for the decision carry the action digest (arguments_hash), approver (approver_did), policy version, and the policy_decision_id / approval_request_id / approval_resolution_id (ADR-0030 section 7), reusing the existing AuditEntry assurance fields.

Scope and deferred work

• No changes to the approval_protocol foundation package (kept separate, per the issue).
• The legacy approval-handler-only path is unchanged when no coordinator is configured (fully backward compatible).
• The synchronous handler bridge drives a single stage-0 entry. Multi-stage chains and async / webhook transport remain future work (ADR-0030 steps 3 and 4).

Tests

tests/test_govern_approval_coordinator.py (8 tests): approve runs the tool; reject denies; an unpermitted identity denies; zero-TTL denies (fail-closed timeout); audit linkage carries the protocol ids; the action digest is bound to parameters; non-approval actions are unaffected; the legacy path still works without a coordinator. Full govern / governance / approval_protocol suites green (100 passed).

Closes #3067.

Refs #2478.

[github-actions]

🤖 AI Agent: security-scanner — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

No security issues found.

[github-actions]

🤖 AI Agent: code-reviewer — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

TL;DR: 0 blockers, 1 warning. The PR introduces a robust ADR-0030 implementation but has a minor test coverage gap.

#	Sev	Issue	Where
1	Warn	Test coverage for edge cases in ApprovalCoordinator integration could be expanded.	test_govern_approval_coordinator.py

Action items:

1. Expand test cases in test_govern_approval_coordinator.py to cover edge scenarios like invalid approval_chain_id, malformed ActionBinding, and ApprovalProtocolError handling.

| Warnings: fine as follow-up PRs. |

[github-actions]

🤖 AI Agent: breaking-change-detector — API Compatibility

AI-generated review output. Treat it as untrusted analysis and verify before acting.

API Compatibility

Severity	Change	Impact
High	Added new parameters approval_coordinator, approval_chain_id, and approval_ttl_seconds to the govern() function and GovernanceConfig class.	Existing calls to govern() or instantiations of GovernanceConfig without these parameters will fail if positional arguments are used.
High	Modified _handle_approval method to route require_approval decisions through a new _handle_approval_via_coordinator method when approval_coordinator and approval_chain_id are set.	Behavior of require_approval decisions changes when these new parameters are configured, potentially impacting existing workflows.
High	Introduced new private methods _handle_approval_via_coordinator and _build_action_binding in GovernedCallable.	Subclasses or external code relying on overriding or directly accessing _handle_approval may break due to changes in its behavior.
Medium	Added dependency on approval_protocol module and its classes (e.g., ApprovalCoordinator, ActionBinding, ActionTarget).	Changes in the approval_protocol module may now impact the governance package.

[github-actions]

🤖 AI Agent: docs-sync-checker — Docs Sync

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Docs Sync

• GovernanceConfig class in agentmesh/governance/govern.py -- missing docstring for new attributes approval_coordinator, approval_chain_id, and approval_ttl_seconds.
• govern() function in agentmesh/governance/govern.py -- missing docstring updates for new parameters approval_coordinator, approval_chain_id, and approval_ttl_seconds.
• README.md -- no evidence of updates to reflect the new approval_coordinator, approval_chain_id, and approval_ttl_seconds configuration options.
• CHANGELOG -- missing entry for the new feature routing require_approval through the ApprovalCoordinator.

[github-actions]

🤖 AI Agent: test-generator — `agentmesh/governance/govern.py`

AI-generated review output. Treat it as untrusted analysis and verify before acting.

agentmesh/governance/govern.py

• test_handle_approval_via_coordinator_unexpected_approval_protocol_error -- Test behavior when ApprovalProtocolError is raised during submit_entry.
• test_handle_approval_via_coordinator_invalid_verdict -- Test behavior when validate_for_execution returns a verdict that is None or not allowed.
• test_handle_approval_via_coordinator_missing_approval_coordinator -- Test behavior when approval_coordinator is not configured but approval_chain_id is set.
• test_build_action_binding_invalid_context -- Test _build_action_binding with invalid or unexpected context structures.

Test coverage for the new ApprovalCoordinator integration is good but could benefit from these additional edge cases.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

[Imran Siddique (imran-siddique)]

Solid ADR-0030 implementation. The coordinator path is fail-closed on both TTL expiry and unpermitted identities, the action digest binds to the full parameter set (tamper-evident), and the audit entries carry all required protocol IDs. Legacy path unchanged when coordinator is not configured -- backward compat preserved. Test suite covers every critical branch. LGTM.

[Imran Siddique (imran-siddique)]

C:/Program Files/Git/rerun

[Ricky Gummadi (Ricky-G)]

Reviewed and happy with this. The coordinator path is fail-closed on both timeout and unpermitted identities, the action digest binds the full parameter set, and the legacy path stays the default when no coordinator is configured, so existing users are unaffected. Imran Siddique (@imran-siddique) has already approved.

One housekeeping ask before merge: please add Closes #3067 to the description so the tracker closes automatically. The title references it, but the closing keyword needs to be in the body to take effect.

Good to merge once that is in.

[Carlos Hernandez (carloshvp)]

Thanks Ricky Gummadi (@Ricky-G). Added Closes #3067 to the description so the tracker auto-closes on merge. Ready to go.