feat: adds deadline/timeout support to IrrevocableContext

[S T (staheri14)]

Summary

Extends IrrevocableContext with deadline/timeout support and a couple of small conveniences, ported from the older sanaz/context-with-tokio branch (ThrowableContext) and adapted to main's current API and conventions.

These features had never landed in main: PR #65 introduced a deliberately simplified context (cancellation + irrecoverable-error propagation only). This PR adds back the deadline capability as a purely additive change — a context created with new behaves exactly as before.

What's added

• with_timeout(span, tag, dur) — root context that expires after dur
• deadline() / is_deadline_exceeded() — inspect the deadline
• run() now races the operation against the deadline and cancellation (whichever fires first)
• err() — Some(..) if the context is cancelled or past its deadline
• with_cancel(tag) — returns a child context plus a closure that cancels it

Child contexts inherit the parent's deadline.

Deliberately excluded

The Go-style value bag (with_value/value::<T>) from the source branch was left out — no consumer in the codebase, and it was intentionally dropped by #65's simplification.

Not yet wired

This is a library-level addition. BaseNode::search_by_id still blocks on rx.recv() without a deadline; wiring it to run under ctx.run(..) is a suggested follow-up.

Testing

• cargo test — full suite passes (context module 10/10, incl. 3 new tests for deadline/err/with_cancel)
• cargo clippy --all-targets --all-features -- -D warnings -D deprecated — clean

🤖 Generated with Claude Code

[S T (staheri14)]

Claude (@claude) review this PR

[claude]

Claude finished S T (@staheri14)'s task —— View job

---

PR Review: Add deadline/timeout support to IrrevocableContext

• Read repo conventions (no CLAUDE.md present)
• Read changed files (mod.rs, context_test.rs)
• Analyze deadline/timeout implementation
• Review tests
• Post review feedback

Overall this is a clean, well-documented, purely additive change. The new path is untouched, docs are updated accurately, and the new tests cover the core new behaviors. Nice work. A few points below, mostly minor / worth-considering.

---

Correctness

1. run() re-computes remaining and races sleep(remaining) rather than sleeping to the absolute deadline. mod.rs:128-131

let remaining = deadline.saturating_duration_since(Instant::now());
tokio::select! {
    ...
    _ = tokio::time::sleep(remaining) => Err(anyhow::anyhow!("context deadline exceeded")),

This is correct, but note there's a subtle behavioral difference from a run that's called after the deadline already passed: saturating_duration_since yields 0, and sleep(Duration::ZERO) is still polled once and resolves ~immediately — but it competes in select! with future. If future is also immediately ready (e.g. async { Ok(x) }), select! picks a branch pseudo-randomly, so an already-expired context could still return Ok for a ready future. Consider short-circuiting at the top:

if let Some(err) = self.err() {
    return Err(err);
}

This also makes run() consistent with err() and avoids racing an already-exceeded deadline.

2. Deadline error string differs between run() and err(). run() returns "context deadline exceeded" (mod.rs:131) and err() returns "context deadline exceeded" (mod.rs:112) — these match, good. But both are constructed ad-hoc in multiple places. Consider centralizing (e.g. small private helpers deadline_err() / cancelled_err()) so the strings can't drift, since tests assert on .contains("deadline exceeded") / .contains("context cancelled").

---

Design considerations

3. with_timeout cannot be combined with child-derived timeouts / shorter child deadlines. A child always inherits the parent's deadline verbatim (mod.rs:72). There's no way to give a child a shorter deadline than its parent (Go's WithTimeout on a child intersects deadlines). Not required for this PR, but worth a doc note or follow-up, since the PR frames this as Go-context-like.

4. with_cancel returns impl Fn() that is Clone/Send? The returned closure captures a CancellationToken (which is Clone + Send + Sync). The impl Fn() return type erases those bounds, so callers can't move it across threads/store it generically without them being inferred. If cross-thread cancellation is an intended use, consider impl Fn() + Send + Sync + Clone. Minor, only matters if a consumer needs it.

---

Tests

5. Timing-based tests may be flaky under load / CI. test_run_respects_deadline (10ms deadline vs 200ms op) and test_is_deadline_exceeded (5ms deadline, sleep 20ms) rely on wall-clock timing. tokio offers #[tokio::test(start_paused = true)] + tokio::time::advance() for deterministic time control, which would make these robust. The margins here are generous enough to likely be fine, but paused-time is the more reliable pattern.

6. Missing test: cancellation winning over deadline in run(). There's a test for deadline expiry and one for cancellation (no-deadline path), but none exercises the Some(deadline) branch where cancellation fires before the deadline. Worth adding to cover that select! arm.

7. No test for the short-circuit case in point 1 (calling run() on an already-expired context). If you adopt the short-circuit, add a test asserting Err even for an immediately-ready future.

---

Nits

• mod.rs:104 is_some_and — good, modern idiom.
• The err() ordering (cancelled checked before deadline) is reasonable; just make sure that priority is intentional and documented if a consumer relies on it.

---

Note: I was unable to run cargo test / cargo clippy in this environment (command not in allowed tools), so I could not independently verify the PR's stated green results. The reasoning above is from static review.
• Branch: sanaz/context-extras-eval

[S T (staheri14)]

Thanks for the review Claude (@claude) — went through all points. Summary of what was addressed vs. deferred:

Addressed in 6e0f90a:

• 1 + 7 — run() now short-circuits via err() before the select!, so an already-cancelled/expired context can't return Ok by racing an immediately-ready future. Added test_run_short_circuits_when_deadline_already_passed.
• 6 — added test_run_cancellation_beats_deadline, which cancels while run() is in-flight (via tokio::join!) to exercise the Some(deadline) arm's cancelled() branch (not the short-circuit).
• 4 — with_cancel now returns impl Fn() + Send + Sync + Clone so the trigger can cross threads/tasks and be stored.

Verification: context module 12/12, full suite 69 pass, cargo clippy --all-targets --all-features -- -D warnings -D deprecated clean.

Deferred to #83 (non-blocking):

• 2 — centralize the deadline/cancelled error strings (DRY).
• 3 — child cannot take a shorter deadline than its parent (Go intersects); documented as a follow-up.
• 5 — convert timer-based tests to start_paused = true + time::advance() for determinism.

[TheP2P (thep2p)]

One small thing worth flagging before run() becomes the path everything funnels through. The span guard is held across the .await inside the select!, which makes the returned future !Send and can leak the span onto other tasks while the operation is suspended. It's pre-existing, but since run() is now the deadline aware entry point, better to move to .instrument() here before we wire search_by_id onto it. Not blocking, approving.

[S T (staheri14)]

One small thing worth flagging before run() becomes the path everything funnels through. The span guard is held across the .await inside the select!, which makes the returned future !Send and can leak the span onto other tasks while the operation is suspended. It's pre-existing, but since run() is now the deadline aware entry point, better to move to .instrument() here before we wire search_by_id onto it. Not blocking, approving.

Thanks, addressed 2748462