feat: add requires_command guard sentinel and check_preconditions() hook

[maisim]

Background

Two related reliability gaps existed in the fact-collection layer:

1. The old shell guard for requires_command used:

   ! command -v <bin> >/dev/null || (<fact-command>)
   

   When the binary is absent this produces no stdout, making it impossible to distinguish "binary not installed" from "binary installed but returned no data". Both cases silently return default(), masking real errors.

2. There was no way for a fact to express runtime prerequisites beyond "this binary must exist" — e.g. ZFS datasets cannot be listed when the zfs kernel module is not loaded, even if the zfs binary is present.

Changes

Replace the old ! … || guard with an explicit if/then/else that emits a unique marker when the binary is absent:

if command -v <bin> >/dev/null 2>&1; then
    (<fact-command>);
else
    echo '##PYINFRA_NOCMD##';
fi

The marker _MISSING_COMMAND_MARKER = "##PYINFRA_NOCMD##" is detected in _get_fact and converted into a MissingCommandError exception so callers can tell the two cases apart.

FactError
└── FactNotCollected          # base: fact skipped, not a data error
    ├── MissingCommandError   # requires_command binary absent
    └── FactPreconditionError # check_preconditions() returned a reason

All three are exported from pyinfra.api.

Both exceptions obey the deploy phase in get_fact():

• Prepare phase: silently return default(). The binary / module may simply not be installed yet; a later operation will install it.
• Execute phase: re-raise. The binary / module should already be present; its absence indicates incorrect ordering in the deploy.

New optional override on FactBase:

def check_preconditions(self, state, host) -> str | None:

Return None (or no explicit return) to proceed, or a reason string to skip with context. The framework raises FactPreconditionError automatically — fact authors never need to import exception classes.

As an example:

• ZfsPools added with requires_command = "zpool".
• ZfsDatasets.check_preconditions() uses the existing server.KernelModules fact to verify the zfs kernel module is loaded before attempting zfs get -H all — a real-world scenario where requires_command alone is not sufficient.

Tests

• tests/test_api/test_api_facts_missing_command.py — phase-aware behaviour for MissingCommandError and the ##PYINFRA_NOCMD## sentinel.
• tests/test_api/test_api_facts_preconditions.py — phase-aware behaviour for FactPreconditionError via check_preconditions().

• Pull request is based on the default branch (3.x at this time)
• Pull request includes tests for any new/updated operations/facts
• Pull request includes documentation for any new/updated operations/facts
• Tests pass (see scripts/dev-test.sh)
• Type checking & code style passes (see scripts/dev-lint.sh)

[DonDebonair]

I love this PR! Left a few small suggestions.

[DonDebonair]

Given the return value, I would make the function name a bit more descriptive. Something preconditions_met?

[maisim]

That seems relevant to me.

[DonDebonair]

Why allow None as a return value? I think the signature is cleaner if it's just bool | tuple[Literal[False], str]

[maisim]

The goal if to not have to return True "manually" if all is ok :

    @override
    def preconditions_met(self, state, host):
        from pyinfra.facts.server import KernelModules

        modules = host.get_fact(KernelModules) or {}
        if "zfs" not in modules:
            return False, "kernel module 'zfs' is not loaded"
        return True # <-- implies to always add return True at the end of the method

Explicit is better than impliciit, maybe we can do like that

Another way if to just return the reason, a string, and not False or a tulple `False, "resaon", because if we return anithing, it's because there is a problem

    @override
    def preconditions(self, state, host):
        from pyinfra.facts.server import KernelModules

        modules = host.get_fact(KernelModules) or {}
        if "zfs" not in modules:
            return  "kernel module 'zfs' is not loaded"

It might be less intuitive
What do you think ?

[DonDebonair]

That is actually a good point. Suggestion: rename the method to check_preconditions(...) and make it return str | None. If it returns a string, some preconditions were not met and the return value gives more details. If nothing (None) is returned, all is well.

[DonDebonair]

Looks good to me. @Fizzadar ready to merge. Recommend not squashing the commits because they represent 2 different (but related) features.

[Fizzadar]

I love this PR, this is vastly improved vs. the current behavior. My concern is with the breaking change to raise during execution. I think this is quite likely to break existing working deploys that rely on the defaulting behaviour, something like:

# Executed on both webserver.net + dbserver.net, default (due to missing command) on webserver is silently skipped
if host.get_fact(postgres.Databases):
   ...

At the same time, I don't want this stuck waiting for v4. What about logging, rather than raising, during execution phase (perhaps with config to enable raising?).

@maisim @DonDebonair keen for your thoughts!

[maisim]

Made the change here, It does not raise anymore, just log a warning.

[Fizzadar]

Awesome PR, huge QOL improvement! Thank you @maisim 🙏