Remove empty executionData early-return from MoiraiDelegate; guard in PolicyManager

Post-audit design: PolicyManager now gates _execute behind a non-empty
executionData check in installWithSignature and replaceWithSignature.
Policies can therefore assume onExecute is always a genuine execution
intent and no longer need the empty-data no-op guard themselves.

- PolicyManager.installWithSignature: only calls _execute when executionData.length > 0
- PolicyManager.replaceWithSignature: same guard
- MoiraiDelegate._onExecute: remove `if (executionData.length == 0) return` early-return; update NatSpec
- execute.t.sol: remove obsolete test_isExecuted_remainsFalse_afterEmptyDataNoOp and stale comments

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: hannahjin

src/PolicyManager.sol

@@ -282,14 +282,15 @@ contract PolicyManager is EIP712, ReentrancyGuard {
 
     /// @notice Installs a policy using an account signature over the binding, optionally followed by an execution.
     ///
-    /// @dev The signature only authorizes the binding (not the execution). Any `executionData` provided, including empty
-    ///      bytes, is forwarded to the policy's execute hook, which MUST enforce its own execution authorization semantics.
+    /// @dev The signature only authorizes the binding (not the execution). If `executionData` is non-empty it is
+    ///      forwarded to the policy's execute hook, which MUST enforce its own execution authorization semantics.
+    ///      Empty `executionData` is treated as install-only — the execute hook is not invoked.
     ///
     /// @param binding Policy binding parameters authorized by the account (includes `policyConfig`).
     /// @param userSig ERC-6492-compatible signature by `binding.account` over the install typed digest:
     ///      `_hashTypedData(keccak256(abi.encode(INSTALL_POLICY_TYPEHASH, policyId, deadline)))`.
     /// @param deadline Optional timestamp (seconds). If non-zero, the signature is invalid after this deadline.
-    /// @param executionData Policy-defined per-execution payload (may be empty; the policy decides how to handle it).
+    /// @param executionData Policy-defined per-execution payload. If empty, no execution is triggered.
     ///
     /// @return policyId Deterministic policy identifier derived from the binding.
     function installWithSignature(
@@ -306,7 +307,9 @@ contract PolicyManager is EIP712, ReentrancyGuard {
 
         _install(binding);
 
-        _execute(binding.policy, policyId, binding.policyConfig, executionData, msg.sender);
+        if (executionData.length > 0) {
+            _execute(binding.policy, policyId, binding.policyConfig, executionData, msg.sender);
+        }
         return policyId;
     }
 
@@ -376,7 +379,7 @@ contract PolicyManager is EIP712, ReentrancyGuard {
     /// @param userSig ERC-6492-compatible signature by `payload.newBinding.account` over the replacement typed digest:
     ///      `_hashTypedData(keccak256(abi.encode(REPLACE_POLICY_TYPEHASH, account, oldPolicy, oldPolicyId, keccak256(oldPolicyConfig), newPolicyId, deadline)))`.
     /// @param deadline Optional timestamp (seconds). If non-zero, the signature is invalid after this deadline.
-    /// @param executionData Policy-defined per-execution payload (may be empty; the policy decides how to handle it).
+    /// @param executionData Policy-defined per-execution payload. If empty, no execution is triggered.
     ///
     /// @return newPolicyId Deterministic policy identifier for the new binding.
     function replaceWithSignature(
@@ -405,7 +408,9 @@ contract PolicyManager is EIP712, ReentrancyGuard {
 
         _replace(payload);
 
-        _execute(payload.newBinding.policy, newPolicyId, payload.newBinding.policyConfig, executionData, msg.sender);
+        if (executionData.length > 0) {
+            _execute(payload.newBinding.policy, newPolicyId, payload.newBinding.policyConfig, executionData, msg.sender);
+        }
 
         return newPolicyId;
     }

src/policies/MoiraiDelegate.sol

@@ -159,22 +159,20 @@ contract MoiraiDelegate is SingleExecutorPolicy {
     /// @dev Validates config hash, checks single-execution guard, enforces time-lock and/or executor signature,
     ///      then returns the configured calldata as the account call.
     ///
-    ///      When `executionData` is empty the function returns early without touching `executed[policyId]`.
-    ///      This is an intentional no-op: it does NOT consume the one-shot execution lock. Callers who want
-    ///      to trigger the policy must supply non-empty `executionData`.
+    ///      When this hook is called the intention is always a genuine execution — the `PolicyManager` ensures
+    ///      `onExecute` is only invoked when execution is explicitly requested.
     ///
-    ///      For delay-only policies (`executor == address(0)`), the content of `executionData` is
-    ///      ignored entirely — only its non-zero length is checked. Any non-empty bytes trigger execution
-    ///      once the time-lock is met. The actual call parameters (`target`, `value`, `callData`) are always
-    ///      taken from `policyConfig`, not from `executionData`.
+    ///      For executor-required policies (`executor != address(0)`), `executionData` is decoded to extract
+    ///      the executor signature. For delay-only policies (`executor == address(0)`), `executionData` is never
+    ///      inspected — only the time-lock matters. The actual call parameters (`target`, `value`, `callData`)
+    ///      are always taken from `policyConfig`, not from `executionData`.
     function _onExecute(
         bytes32 policyId,
         address account,
         bytes calldata policyConfig,
         bytes calldata executionData,
         address caller
     ) internal override whenNotPaused returns (bytes memory accountCallData, bytes memory postCallData) {
-        if (executionData.length == 0) return (accountCallData, postCallData);
         _requireConfigHash(policyId, policyConfig);
 
         (SingleExecutorConfig memory singleExecutorConfig, bytes memory specificConfig) =

test/unit/policies/MoiraiDelegate/execute.t.sol

@@ -30,7 +30,6 @@ contract ExecuteTest is MoiraiDelegateTestBase {
 
         vm.warp(unlockTime);
 
-        // For delay-only, executionData must be non-empty to bypass the early-return guard.
         assertFalse(policy.executed(policyId));
         policyManager.execute(address(policy), policyId, config, bytes("0x01"));
         assertTrue(policy.executed(policyId));
@@ -108,16 +107,6 @@ contract ExecuteTest is MoiraiDelegateTestBase {
         assertTrue(policy.executed(policyId));
     }
 
-    /// @notice isExecuted remains false after a no-op execute call with empty executionData.
-    function test_isExecuted_remainsFalse_afterEmptyDataNoOp() public {
-        bytes memory config = _buildMoiraiConfig(0, executor);
-        bytes32 policyId = _buildAndInstall(config, 0);
-
-        assertFalse(policy.executed(policyId));
-        policyManager.execute(address(policy), policyId, config, bytes(""));
-        assertFalse(policy.executed(policyId));
-    }
-
     // =============================================================
     // Reverts
     // =============================================================
@@ -142,7 +131,6 @@ contract ExecuteTest is MoiraiDelegateTestBase {
         bytes memory config = _buildMoiraiConfig(unlockTime, address(0));
         bytes32 policyId = _buildAndInstall(config, 0);
 
-        // Must pass non-empty executionData to reach the time-lock check.
         vm.expectRevert(
             abi.encodeWithSelector(MoiraiDelegate.BeforeUnlockTimestamp.selector, block.timestamp, unlockTime)
         );