Cli cancel UX

[benalleng]

This PR builds on #1688 but reverts the completed_at DB column removal.

• Add an auto-cancel mid loop when passing the expiry instead of only on process start.
• print the session ID at the start of the session to better keep track if the user want to immediately cancel.
• Show what sessions have a fallback transaction available from payjoin-cli history for rebroadcasting.

example output. custom timeout of 20s to demonstrate automatic cancel logic

Coded with help from GLM-5.2

Pull Request Checklist

Please confirm the following before requesting review:

• I have disclosed my use of
  AI
  in the body of this PR.
• I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

[benalleng]

@spacebear21 I am most curious of your thoughts on 9b718ac and whether you think this is appropriate for both the sender and receiver

[coveralls]

Coverage Report for CI Build 29342380627

Coverage decreased (-0.2%) to 85.777%

Details

• Coverage decreased (-0.2%) from the base build.
• Patch coverage: 80 uncovered changes across 2 files (63 of 143 lines covered, 44.06%).
• 1 coverage regression across 1 file.

Uncovered Changes

File	Changed	Covered	%
payjoin-cli/src/app/v2/mod.rs	121	42	34.71%
payjoin/src/core/error.rs	10	9	90.0%
Total (5 files)	143	63	44.06%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
payjoin-cli/src/app/v2/mod.rs	1	49.29%

---

Coverage Stats

Relevant Lines:	15981
Covered Lines:	13708
Line Coverage:	85.78%
Coverage Strength:	344.95 hits per line

---

💛 - Coveralls

[bc1cindy]

looks solid

two notes inline on the expiry paths.

the new mid-loop cancel/broadcast and the fallback_available display aren't covered by tests

adding a test driving an expired session through the sender path would lock in the behavior, wdyt?

[bc1cindy]

"Error: Sender session expired" printed here but the fn returns Ok(())

[DanGould]

The issue with automating fallback is perhaps that another session could spend the same coin between sessions. For example if bitcoind was restarted and another payjoin session spent the same coin. Then you might have a fallback automatically broadcast that doesn't pay attention to wallet state? Just seems like some complexity that this would need to take responsibility for. Not completely confident in the motivation and my tendency is to prefer leaving things manual unless the automation is a no brainer default.

[xstoicunicornx]

General nit: a lot of the sender/receiver verbiage has been removed from tracing and print statements, was there a reason for this? Maybe I can take as a followup - as I have been wanting to standardize the printing more in the CLI so that every printed line starts with the session role and ID.

Rather than creating repetitive relay selection/expiration code (and in some places loosing the loop it seems) why not adapt post_via_relay to still be usable in all contexts? For example (code created with Claude Opus 4.8):

/// A request-construction error that can report whether it was caused by
/// session expiry. Implemented by the sender/receiver request-building error
/// types so `post_via_relay` can hand expiry back to the caller (which owns the
/// typestate needed to react) instead of flattening it into `anyhow::Error`.
trait RequestExpiry {
    fn expired(&self) -> bool;
}

impl RequestExpiry for payjoin::send::v2::CreateRequestError {
    fn expired(&self) -> bool { self.is_expired() }
}

impl RequestExpiry for payjoin::receive::v2::CreateRequestError {
    fn expired(&self) -> bool { self.is_expired() }
}

impl RequestExpiry for payjoin::receive::v2::SessionError {
    fn expired(&self) -> bool { self.is_expired() }
}

/// Outcome of building and posting a request via `post_via_relay`. HTTP
/// failures are retried against other relays inside the helper; only session
/// expiry and fatal build errors escape to the caller.
enum RelayPost<T> {
    Posted(reqwest::Response, T),
    Expired,
}

and then:

    async fn post_via_relay<F, T, E>(&self, mut build: F) -> Result<RelayPost<T>>
    where
        F: FnMut(&str) -> std::result::Result<(payjoin::Request, T), E>,
        E: RequestExpiry + Into<anyhow::Error>,
    {
        loop {
            let relay = self.mailroom_manager.choose_relay()?;
            let (req, ctx) = match build(relay.as_str()) {
                Ok(r) => r,
                Err(e) if e.expired() => return Ok(RelayPost::Expired),
                Err(e) => return Err(e.into()),
            };
            match self.post_request(req).await {
                Ok(resp) => return Ok(RelayPost::Posted(resp, ctx)),
                Err(e) => {
                    tracing::debug!("Request to relay {relay} failed: {e:?}");
                    self.mailroom_manager.add_failed_relay(relay);
                }
            }
        }
    }
}

See full (half-reviewed) implementation here in last commit https://github.com/xstoicunicornx/rust-payjoin/tree/pr1700-mod

[benalleng]

Maybe I can take as a followup - as I have been wanting to standardize the printing more in the CLI so that every printed line starts with the session role and ID.

I'm fine dropping the last commit if you want to take this up. I've gotten a bit lost in the sauce having redesigned this PR and I'm not sure what I think is best for the readability

[xstoicunicornx]

Few more questions but overall looking good.

[benalleng]

Got caught up a bit in rebase hell on this one, thanks for being diligent in your reveiws @xstoicunicornx

I think this is good for a re-review

[xstoicunicornx]

A couple more things I noticed. The comment in finalize_expired_receiver is just a suggestion but there are other ways to implement that don't require as much of a refactor.

[xstoicunicornx]

I think this needs to handle sessions that have a fallback tx.

[xstoicunicornx]

Shouldn't we be avoiding closing automatically?

[xstoicunicornx]

utACK 2201bdc

Reviewed and tested the code. This implements expiry handling at every point where expiry could get hit rather than just upon startup.

Will be taking on followup to standardize the messages printed to the user.