cs-bit polarity inverted in compound_wait_check_of() and should_execute_chained() #58

Problem

compound_wait_check_of() and should_execute_chained() in include/rcp/request.hpp implement the cs bit's polarity backwards relative to the enum's own documented mapping and the field's own name.

Evidence

include/rcp/request.hpp:352-368:

enum class CompoundWaitCheck : uint8_t {
    Immediate       = 0, // evaluate the condition against current status right away
    AfterChangeOnly = 1, // only evaluate once the monitored value changes from its current one
};

constexpr CompoundWaitCheck compound_wait_check_of(bool cs) noexcept {
    return cs ? CompoundWaitCheck::Immediate : CompoundWaitCheck::AfterChangeOnly;
}

// should_execute_chained answers, for one chained successor: given its own
// `cs` bit and whether its predecessor finished in error, should this
// successor still run? cs=true means "execute regardless"; cs=false means
// "abort if the predecessor errored". A predecessor that did *not* error
// always permits the successor to run, independent of cs.
constexpr bool should_execute_chained(bool cs, bool predecessor_errored) noexcept {
    return cs || !predecessor_errored;
}

compound_wait_check_of(cs) returns Immediate when cs is true and AfterChangeOnly when cs is false — the exact opposite of what the enum's own inline comments describe (Immediate = 0, AfterChangeOnly = 1) and the opposite of what the field's name implies: cs stands for "conditional-start," so cs = true ("apply the condition") should map to AfterChangeOnly, and cs = false should map to Immediate.

should_execute_chained() has the same directional problem relative to that naming convention.

tests/test_request.cpp:520-534 assert exactly this inverted behavior, so CI is green and the bug is effectively locked in by the test suite rather than caught by it.

should_execute_chained() is reachable from a real public API path: RequestLedger::finalize() → propagate_chain_completion() → should_execute_chained(). compound_wait_check_of() currently has no callers anywhere in the tree outside of its own unit test — so today the bug is latent for compound-wait requests specifically, but live for chained requests, and will become live for compound-wait as soon as any caller wires it up (which is presumably imminent, since the function exists for that purpose).

Why it matters

Any caller that trusts the enum's documented semantics (0 = Immediate, 1 = AfterChangeOnly) or the cs field's "conditional-start" naming will get the wrong wait-check timing or the wrong chained-successor abort/continue decision — silently, since the tests currently bake in and protect the wrong behavior.

Suggested fix

Flip the ternary in compound_wait_check_of() and the boolean logic in should_execute_chained() to match the documented enum values and field semantics, then update tests/test_request.cpp:520-534 to assert the corrected polarity. Worth double-checking against the OPEN Alliance TC18 spec text (extraction §2.4 citation in the surrounding comment) to confirm 0/1 really do mean what the local enum comment says before changing behavior that other RELAY-family implementations may already depend on matching.