refactor(logging): one verification-failure rule for both receive paths

[oferchen]

The rule

receiver.c:1071-1091 is one rule with five decisions in it:

enum logcode msgtype = redoing ? FERROR_XFER : FWARNING;
if (msgtype == FERROR_XFER || INFO_GTE(NAME, 1) || stdout_format_has_i) {
        char *errstr, *redostr, *keptstr;
        if (!(keep_partial && partialptr) && !inplace)
                keptstr = "discarded";
        else if (partial_dir)
                keptstr = "put into partial-dir";
        else
                keptstr = "retained";
        if (msgtype == FERROR_XFER) {
                errstr = "ERROR";
                redostr = "";
        } else {
                errstr = "WARNING";
                redostr = read_batch ? " (may try again)" : " (will try again)";
        }
        rprintf(msgtype, "%s: %s failed verification -- update %s%s.\n",
                errstr, local_name ? f_name(file, NULL) : fname, keptstr, redostr);
}

Severity, emission gate, keptstr, retry suffix and format string. Every one of
them reads state oc keeps in a different place on each of its two receive
paths, which is exactly the shape that drifts when the rule is written twice.

Change

crates/logging/src/verify_failure.rs holds the rule once:

pub fn verification_failure(name: &Path, state: VerifyFailure) -> Option<(LogCode, String)>

VerifyFailure has seven fields and each one is a single upstream variable,
named for it: redoing, read_batch, stdout_format_has_i, keep_partial,
has_partial_path (partialptr != NULL), partial_dir, inplace.
INFO_GTE(NAME, 1) is read inside the function through logging::info_gte, so
that disjunct of the gate is single-sourced too.

crates/logging is the right home and LogCode the right return type: the
crate has no workspace dependencies, so it cannot take transfer's PartialMode
or protocol's MessageCode, and it already owns LogCode - upstream's
enum logcode, with the ErrorXfer/Warning this rule selects between - and
the INFO_GTE lookup. Callers route the code: the network receiver converts it
with the existing MessageCode::from_log_code; the local executor writes it to
whichever stream log.c:313-316 routes that code to.

The network receiver keeps only what is genuinely its own: partial_state,
which projects its session-wide PartialMode onto upstream's three separate
variables. verification_kept_str and reports_verify_warning are deleted.

No behaviour change on the transfer path. The projection is equivalent to the
deleted verification_kept_str on every input:

PartialMode	inplace	before	after
None	false	discarded	discarded
None	true	retained	retained
Partial	any	retained	retained
PartialDir	any	put into partial-dir	put into partial-dir

Tests

Table-driven, in logging, over the pure function.

Seven keptstr rows, each run through both severities:

keep_partial	partialptr	partial_dir	inplace	keptstr
F	F	F	F	discarded
T	F	F	F	discarded
F	F	T	F	discarded
T	T	T	F	put into partial-dir
T	T	F	F	retained
F	F	F	T	retained
F	F	T	T	put into partial-dir

The two bold rows are unreachable from the command line - upstream sets
keep_partial whenever --partial-dir is given, and rejects
--inplace --partial-dir outright - but they are what pin the if/else-if
ORDER. Rewriting the chain as three independent predicates passes every
reachable row and fails exactly these two.

Four gate rows cover all three disjuncts of receiver.c:1072: silent by
default, reported under INFO_GTE(NAME, 1), reported under
stdout_format_has_i, and reported unconditionally when the severity is
FERROR_XFER. Plus: the ERROR form carries no retry suffix even under
read_batch - which catches a hoist that appends redostr before checking
redoing.

On the transfer side the old keptstr unit test is replaced by two that cover
what remains there: the PartialMode projection, and an end-to-end check that
every keptstr is still reachable through this receiver's own types.

Second call site

The local-copy emit site is warn_verification_failed in #7235, which is not
merged yet. It hardcodes keptstr = "retained", redostr = " (will try again)"
and the severity, and derives the first from local append state rather than from
inplace - a third copy of the rule in prose form. It also spells
stdout_format_has_i as is_itemize_active() where the receiver spells it
out_format_forwards_i. Delegating it is a follow-up on that PR; this one
lands the shared rule and the network call site.

Verification

Run on the aarch64 Linux host, since GitHub Actions is not scheduling. Exit
codes checked directly, never through a pipe.

• cargo fmt --all -- --check - exit 0
• cargo clippy --locked --workspace --all-targets --all-features --no-deps -- -D warnings - exit 0
• cargo nextest run --workspace --all-features --no-fail-fast

A clippy::nonminimal_bool finding on the keptstr chain was fixed by naming
the positive condition (survives = inplace || (keep_partial && has_partial_path))
and negating once - not by a waiver.

Does this RETIRE the drift risk, or only REDUCE it?

Retired, as of this PR, conditionally. Stated plainly rather than assumed:

On current master plus this branch there is exactly one implementation and one
call site. verification_kept_str and reports_verify_warning are deleted;
the network receiver computes no part of the rule locally. What remains in
transfer is partial_state, which is a projection of oc's own PartialMode
onto upstream's three variables - not a second copy of the decision. A
single-site test therefore suffices, and no cross-implementation test is
required or possible today, because there is nothing to compare against.

The condition: #7235 has not merged. It adds the local-copy emit site, and
as written it hardcodes keptstr, redostr and the severity, deriving the
first from local append state rather than from inplace. If it lands that way,
the rule is back in two places and the risk is only reduced - at which point a
cross-implementation test feeding both sites the same input becomes mandatory,
and I will own it. The exact delegating call site has been handed to that PR's
author to fold into the rebase in flight, so the intended end state is one
implementation with two call sites.

Verified per the standing question: keptstr's inplace input on the transfer
path is the post-#7243 implied flag. The receiver passes pending.is_inplace,
which is resolve_use_inplace(config.write.inplace, ..), and
ServerConfig::apply_append_implies_inplace sets that flag for --append. So an
append whose verification fails reports its update "retained", not "discarded",
and it reaches that string through the same input upstream uses.

Boundary with #377

• This PR owns what the message says and which LogCode it carries.
• Allow cloning session handshake parts #377 owns where a LogCode routes, mirroring rwrite (log.c:251).

verification_failure returns Option<(LogCode, String)> and does no routing,
so the funnel absorbs it without rework: callers keep whatever sink they use
today and #377 collapses the sinks later. That is why the return type is
LogCode and not an already-chosen stream.

The gate input must stay format-derived

options.c:2345-2358 feeds one variable from two sources, and -i rewrites
stdout_format to "%i %n%L":

if (stdout_format) {
        if (am_server && log_format_has(stdout_format, 'I'))  stdout_format_has_i = 2;
        else if (log_format_has(stdout_format, 'i'))          stdout_format_has_i = itemize_changes | 1;
} else if (itemize_changes) {
        stdout_format = "%i %n%L";
        stdout_format_has_i = itemize_changes;
}

So a format-derived value catches both -i and a bare
--out-format='%i%n' given without -i; an -i boolean misses the second.
The transfer receiver's out_format_forwards_i is the faithful spelling and is
what feeds this field. The engine's is_itemize_active() (a bool from -i) is
narrower - a real divergence, not a stylistic one. The field is documented
accordingly so nobody "simplifies" it back to the -i bool.

Also documented on the field: upstream's stdout_format_has_i is a tri-state
(0/1/2 - 2 when am_server and the format carries %I, and itemize_changes
is a counter at options.c:1581, so -ii also yields 2). Several upstream sites
test > 1 (generator.c:583,1010,1138, hlink.c:400, log.c:832). This gate is
not one of them - receiver.c:1072 tests plain truthiness - so bool is exact
here, and the doc says so explicitly to stop a future caller reusing the field at
a > 1 site and silently losing the -i/-ii distinction.

Checked and clean: oc does not conflate stdout_format_has_i with
logfile_format_has_i (receiver.c:644's server-side variable);
generator/protocol_io.rs:606 already distinguishes them.