fix(ws-13): verify cancellation from turn state

[serrrfirat]

Context

Split out from the follow-up fixes for #3648 so the original WS13 cancellation accessor PR can stay focused.

What changed

• ThreadCheckpointLoopExitEvidencePort now reads durable turn state for the claimed run.
• LoopExit::Cancelled evidence is accepted only when the run is durably CancelRequested.
• Added a regression test for a cancellation exit backed by durable CancelRequested state.

Validation

• cargo test -p ironclaw_reborn loop_exit_applier

Stack

Base: #3648 / arch/ws-13
Next: codex/ws13-live-cancel-wiring

[gemini-code-assist]

Code Review

This pull request integrates TurnStateStore into the LoopExitApplier logic to dynamically verify if a cancellation has been requested for a specific run. It replaces the previous static return in is_cancellation_observed with a lookup to the state store and updates associated tests and instantiation sites. Feedback suggests expanding the cancellation check to include the Cancelled state as well as CancelRequested to ensure idempotency and better handle retries after a worker crash.

[gemini-code-assist]

To support idempotent retries and robust recovery, the check should also accept runs that are already in the Cancelled state. If a worker crashes after successfully applying a cancellation but before completing its task, a subsequent retry should still consider the cancellation as 'observed' rather than failing with a protocol violation.

Suggested change

	Ok(state.status == TurnStatus::CancelRequested)
	Ok(state.status == TurnStatus::CancelRequested || state.status == TurnStatus::Cancelled)

References

1. When managing job or task states, distinguish between active and terminal states (like Cancelled) to ensure robust recovery and idempotency during retries.

[Copilot]

Pull request overview

This PR tightens WS-13 cancellation correctness by requiring durable turn-run state (CancelRequested) before accepting LoopExit::Cancelled evidence, and introduces a host-facing cancellation observation port that the canonical executor consults at cooperative boundaries.

Changes:

• Update ThreadCheckpointLoopExitEvidencePort to read durable run state and only accept cancellation when the run is CancelRequested.
• Add a LoopCancellationPort + LoopCancellationSignal to the host contract and wire it through loop-support + Reborn host adapter plumbing.
• Add/enable regression tests verifying cancellation short-circuits execution and that cancellation exits are backed by durable state.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
crates/ironclaw_turns/tests/agent_loop_host_contract.rs	Updates host contract test host to implement the new cancellation port.
crates/ironclaw_turns/src/run_profile/mod.rs	Re-exports the new cancellation port/signal types from the run-profile API.
crates/ironclaw_turns/src/run_profile/host.rs	Introduces LoopCancellationPort and LoopCancellationSignal, and adds the port to AgentLoopDriverHost.
crates/ironclaw_reborn/tests/planned_driver_e2e.rs	Adds an end-to-end test asserting planned driver short-circuits on host cancellation.
crates/ironclaw_reborn/tests/loop_driver_host.rs	Updates evidence port construction to pass the new TurnStateStore dependency.
crates/ironclaw_reborn/src/turn_runner/tests/mod.rs	Updates the stub host to implement the cancellation port.
crates/ironclaw_reborn/src/loop_exit_applier/tests/mod.rs	Adds a regression test for accepting cancellation evidence only with durable CancelRequested state; adds a static TurnStateStore test double.
crates/ironclaw_reborn/src/loop_exit_applier.rs	Makes cancellation evidence verification consult durable run state via TurnStateStore.
crates/ironclaw_reborn/src/loop_driver_host.rs	Wires per-run cancellation observation into the Reborn host via a RunCancellationFactory and RunStateLoopCancellationPort.
crates/ironclaw_loop_support/src/lib.rs	Exposes the new cancellation port/factory utilities from loop-support.
crates/ironclaw_loop_support/src/cancellation_port.rs	Adds run-scoped cancellation handle/port and a factory abstraction (with tests).
crates/ironclaw_loop_support/Cargo.toml	Adds chrono + parking_lot dependencies needed for cancellation signal storage/locking.
crates/ironclaw_agent_loop/tests/deferred_followups.rs	Enables/implements the prior placeholder integration test for executor-level cancellation short-circuiting.
crates/ironclaw_agent_loop/src/test_support/mod.rs	Extends the mock host builder to supply a cancellation signal and implements LoopCancellationPort.
crates/ironclaw_agent_loop/src/executor.rs	Adds cooperative cancellation checks at boundary points and produces cancellation exits with a final checkpoint when possible.
crates/ironclaw_agent_loop/Cargo.toml	Adds dev-dependencies used by new/updated tests (chrono, futures).
Cargo.lock	Records new dependency resolutions from the added crates/features.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[henrypark133]

Review — fix(ws-13): verify cancellation from turn state

No blockers. Approve.

Evidence gate — is_cancellation_observed correctly reads get_run_state and returns true only on CancelRequested. Logic is sound.

TOCTOU — read-then-accept window is inherent in cooperative check design. State machine prevents double-transition. Acceptable.

CancelCheck enum — private module-local sentinel, not a duplicate of any exported type.

Test coverage — regression test covers the gate path. Executor-level and e2e integration tests cover cooperative boundary detection.

[henrypark133]

Caveman review findings

1🔴 2🟡 1❓

• 🔴 crates/ironclaw_agent_loop/src/executor.rs (~lines 1060–1074): when checkpoint fails and require_final_checkpoint = false, the error is swallowed with Err(_) and a CancelCheck::Exit is returned. A non-checkpoint error (e.g. AgentLoopExecutorError::HostUnavailable) surfacing during the cancel-path checkpoint will be silently treated as a permissive-profile checkpoint failure and produce a checkpoint-free Cancelled exit instead of propagating. Fix: match on Err(AgentLoopExecutorError::CheckpointFailed { .. }) specifically; re-return other variants.
• 🟡 crates/ironclaw_loop_support/src/cancellation_port.rs:25–37: early-exit at line 25 returns without taking the lock. fired.store(true) at line 37 happens after signal_lock = Some(signal) write but the AtomicBool and RwLock are independent objects. On x86 TSO this works; on weaker arches a reader observing fired == true via Ordering::Acquire in observe_cancellation could see None in signal_lock. Fix: store fired inside the write-lock scope before releasing, or document the x86-TSO dependency.
• 🟡 crates/ironclaw_agent_loop/src/executor.rs:944–955 cancelled_after_checkpoint: hardcodes LoopCancelledReasonKind::HostCancellation via cancelled_exit. Dual path with checkpoint_and_exit_if_cancelled means future expansion of LoopCancelledReasonKind won't be picked up here. Currently acceptable since all reasons map to HostCancellation, but unify or mark.
• ❓ crates/ironclaw_reborn/src/loop_exit_applier.rs:257–268 is_cancellation_observed: returns Ok(state.status == TurnStatus::CancelRequested). If the cancel signal arrived recently but the DB row hasn't transitioned (Running), evidence returns false, and LoopExit::Cancelled is treated as interrupted_unexpectedly and fails the run. Is this race acceptable, or is there a lease/heartbeat guarantee that status is always CancelRequested before the runner produces LoopExit::Cancelled?

[henrypark133]

Code Review — fix(ws-13): verify cancellation from turn state

Overview

ThreadCheckpointLoopExitEvidencePort gains a TurnStateStore dependency. is_cancellation_observed now reads the durable run state via get_run_state and accepts LoopExit::Cancelled evidence only when state.status is CancelRequested or Cancelled. Three tests added: two new positive-path tests covering both accepted statuses, plus a StaticTurnStateStore test double. Two callsites updated to thread the new dependency through.

Correctness

• ✅ Predicate widening to CancelRequested | Cancelled addresses the worker-crash retry idempotency hole flagged by gemini-code-assist. Without it, a retry after the row transitioned to terminal Cancelled would observe false and force interrupted_unexpectedly failure.
• ✅ TurnStatus::Cancelled is correctly terminal per is_terminal() in crates/ironclaw_turns/src/status.rs. No new terminal-handling logic needed.
• ✅ Constructor signature change cascaded to both production callsites (tests/loop_driver_host.rs:942, :5257) and to the test helper text_checkpoint_evidence at tests/mod.rs:385. No missed callsites.
• ⚠️ StaticTurnStateStore::get_run_state (tests/mod.rs:435) asserts request.scope == self.state.scope and request.run_id == self.state.run_id. If a future test seeds a state whose scope/run_id differ from the claimed run, the panic message will be a bare assert_eq! failure. Low-risk in test-only code.
• ❓ TOCTOU between the durable-state read and the runner's transition is inherent in cooperative cancellation. State-machine prevents double-transition. Already accepted in prior review.

Style / Conventions

• ✅ Uses matches! for multi-variant predicate — idiomatic, matches existing patterns elsewhere in loop_exit_applier.rs.
• ✅ Import additions grouped correctly; no pub use re-exports.
• ✅ Tests use #[tokio::test] and Arc::new(...) test doubles consistent with sibling tests.
• ✅ No .unwrap()/.expect() in production paths; only .expect(\"applied\") in tests, permitted by .claude/rules/error-handling.md.
• ⚠️ The two new tests thread_checkpoint_evidence_accepts_durable_cancel_requested_run and thread_checkpoint_evidence_accepts_durable_cancelled_run are byte-for-byte identical except the seeded TurnStatus. Acceptable as-is since the two assertions enforce both directions of the matches! predicate; a table-driven helper would obscure regression coverage.

Test Coverage

• ✅ 17/17 in loop_exit_applier::tests (was 16 before this PR; +1 for the new Cancelled test).
• ✅ Both accepted statuses (CancelRequested, Cancelled) covered by separate #[tokio::test]s — predicate has full branch coverage.
• ❌ Missing negative test: no test asserts is_cancellation_observed returns false (and therefore the applier surfaces interrupted_unexpectedly) when the durable status is Running, Queued, BlockedApproval, or Completed. Without this, a regression that accidentally widens the predicate further (e.g. to _ => true) would not be caught. Recommend a third test seeding TurnStatus::Running and asserting evidence rejection. Not a merge blocker — the negative behavior is the pre-existing default — but worth tracking.

Performance

• One additional get_run_state call per LoopExit::Cancelled evaluation. This is on the cancel path, not the hot path; cost is irrelevant.
• StaticTurnStateStore clones the state on each get_run_state (Ok(self.state.clone())). Acceptable in test code.

Security

• ✅ No new boundary: TurnStateStore is an internal trait, scope and run_id are typed newtypes per .claude/rules/types.md.
• ✅ No raw strings, no String comparisons, no format!(\"...\") predicate building.
• ✅ Evidence-port write path unchanged — only the read predicate widened.

Out-of-scope items (tracked elsewhere)

Comments mirrored to upstream stack PRs:

• 🔴 executor.rs cancel-path Err(_) swallows non-checkpoint errors — tracked on fix(ws-13): ack drained inputs before cancel exit #3686 (fix(ws-13): ack drained inputs before cancel exit #3686 (comment)).
• 🟡 cancelled_after_checkpoint hardcoded HostCancellation reason — tracked on fix(ws-13): ack drained inputs before cancel exit #3686 (same comment).
• 🟡 RunCancellationHandle::request AtomicBool + RwLock ordering on weak memory architectures — tracked on fix(ws-13): wire live cancellation into host #3685 (fix(ws-13): wire live cancellation into host #3685 (comment)).

Recommendation

APPROVE. No blockers. The single suggestion (add a negative test for Running status to lock the rejection branch) is a follow-up nit, not a merge gate. The PR is focused, minimal, well-tested, and addresses the only in-scope finding from the prior review round.