This is not really nice.
It would be better if DaemonNode could be used as a "normal node", instead of patching the DaemonNode and modifying the DirectNode to work in the same way.
Probably, this is not possible using XML-RPC. Maybe we could use a different IPC mechanism?

Yeap, I agree. But the problem isn't the IPC mechanism, really. Graph API for actions in rclpy departs from the pattern that the rest follow and on which NodeStrategy implementation rests upon. On a regular node, you'd simply call rclpy.action.get_action_*(node, *args), but those simply forward down to the C layer (see here). The pattern doesn't lend itself well to polymorphism. That's why I made DirectNode look like NodeStrategy, and not the other way around.

I have yet to come up with a way to do this nicely and transparent for those who write API like that of rclpy.action. We could make our C calls through the handle, like:

def get_action_client_names_and_types_by_node(
    node: Node,
    remote_node_name: str,
    remote_node_namespace: str
) -> List[Tuple[str, List[str]]]:
    get_action_client_names_and_types_by_node = node.handle.bind(
        _rclpy_action.rclpy_action_get_client_names_and_types_by_node)
    return get_action_client_names_and_types_by_node(
        remote_node_name, remote_node_namespace)

It isn't pretty, but it gives us a chance to proxy calls.

Tagging @jacobperron @wjwwood, both probably have thoughts on this.

I like that proposal.
It's much more work though, but the result is cleaner.

I agree that we could be more consistent with the API in rclpy. I think any refactoring in rclpy can be left as part of a more thorough design review. For now, what if we just add a couple action specific functions to the DirectNode API?
I think it's easier to understand than inspecting function signatures, e.g.

class DirectNode:

    # ...

    def get_action_names_and_types():
        return rclpy.action.get_action_names_and_types(self.node)

    def get_action_client_names_and_types_by_node(node_name, node_ns):
        return rclpy.action.get_action_client_names_and_types_by_node(self.node, node_name, node_ns)

     # ... etc

I was trying to not do that :). But I'm fine with being explicit. See c0a78fe.

I understand, but I think the diff is easier to parse with override functions (obviously just an opinion). The TODO still holds in any case.

Why are these changes needed and how are they related to this PR?

Those changes slipped in, my bad. These tests flaked me once locally, and later I forgot to take decorators out. We can split into their own PR if you'd like. Or drop them, truth is I cannot consistently reproduce the flake -- and I highly doubt it had anything to do with this patch.

Yeah, splitting sounds reasonable since nobody will expect this change in the squashed commit after the fact.