secsi: T3 / T4 enforcement moved into the FSM

The SECS-I Protocol FSM now enforces T3 (reply timeout) and T4
(inter-block timeout) directly, instead of leaving them as
upper-layer hooks.

T3: on complete_send, if the block we just acked had W=1, record its
system_bytes in awaiting_reply_sys_ and emit ActionStartTimer{T3}.
deliver_recv cancels T3 when a block arrives whose system_bytes
match the outstanding request.  EventTimeout{T3} aborts the FSM with
"T3 reply timeout".

T4: deliver_recv emits ActionStartTimer{T4} whenever the delivered
block has end_block=false.  The next block's deliver_recv cancels
the timer; EventTimeout{T4} aborts with "T4 inter-block timeout".

abort() now also cancels T3/T4 and clears the tracking state.

Test changes:
  - Old "T3/T4 are FSM-level no-ops" test → REPLACED by four new
    tests: T3 arm+expire, T3 arm+matching-reply cancels, T4
    arm+expire, T4 arm+next-block cancels.
  - Two new observer accessors on Protocol (awaiting_reply,
    awaiting_next_block) so the tests can assert tracking state
    without poking internals.

COMPLIANCE.md §1a: T3 + T4 rows go ⬜ → ✅.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

COMPLIANCE.md

@@ -57,8 +57,8 @@ Legend:
 | RTY retry counter                     | ✅     | E4 §10.2 | Per-block retry budget, exhaust → ActionRaiseError. |
 | T1 inter-character timer hook         | ✅     | E4 §10.1 | Drained in `RecvBlock`; host wires the actual asio timer. |
 | T2 protocol timer hook                | ✅     | E4 §10.1 | Triggers a retry from any send state. |
-| T3 reply timer                        | ⬜     | E4 §10.1 | Driven by the upper layer (same as HSMS). |
+| T3 reply timer                        | ✅     | E4 §10.1 | FSM tracks system_bytes of outstanding W=1 primaries; arms T3 on send-complete, cancels on matching reply, aborts on expiry. |
-| T4 inter-block timer                  | ⬜     | E4 §10.1 | Multi-block message-gap; FSM emits hook events. |
+| T4 inter-block timer                  | ✅     | E4 §10.1 | FSM arms T4 when a block delivers with end_block=false; cancels when the next block arrives, aborts on expiry. |
 | Master/slave contention resolution    | ✅     | E4 §7.1.4 | Slave yields on simultaneous ENQ; master holds. |
 | Serial port wiring (asio)             | ⬜     | —        | FSM is IO-free; serial integration is a wiring follow-up. |
 | TCP tunnel for testing                | ✅     | —        | `secsi::TcpTransport` wraps the FSM behind an asio TCP socket; mirrors secsgem-py's `secsitcp/`. |

include/secsgem/secsi/protocol.hpp

@@ -98,6 +98,11 @@ class Protocol {
   // Test-only: peek the queue of blocks waiting to send.
   std::size_t pending_send_size() const { return send_queue_.size(); }
 
+  // Test/observer: whether we're currently awaiting a reply (T3 arm)
+  // or the next block of a multi-block recv (T4 arm).
+  bool awaiting_reply() const { return awaiting_reply_sys_.has_value(); }
+  bool awaiting_next_block() const { return awaiting_next_block_; }
+
  private:
   // Start of one send transaction (when send_queue_ non-empty and we're Idle).
   void begin_send(std::vector<Action>& out);
@@ -122,6 +127,18 @@ class Protocol {
   // --- receive-side state ---
   std::vector<uint8_t> recv_buf_;          // bytes collected since EOT
   std::size_t recv_expected_ = 0;          // length byte + payload + checksum
+
+  // --- transaction-level state (T3 / T4) ---------------------------------
+  // T3: when we send a block with W=1, we expect a reply block whose
+  // system_bytes match.  We track the outstanding system_bytes and arm
+  // T3 in complete_send; deliver_recv cancels it when the matching reply
+  // arrives, or T3 expires and we raise an error.
+  std::optional<uint32_t> awaiting_reply_sys_;
+  // T4: when a recv block delivers with end_block=false we expect at
+  // least one more block in the same message.  We arm T4 in
+  // deliver_recv and cancel when the next block arrives (or T4 expires
+  // and we abort).
+  bool awaiting_next_block_ = false;
 };
 
 const char* state_name(Protocol::State s);

src/secsi/protocol.cpp

@@ -40,6 +40,17 @@ void Protocol::retry_send(std::vector<Action>& out) {
 
 void Protocol::complete_send(std::vector<Action>& out) {
   out.push_back(ActionCancelTimer{Timer::T2});
+  // If the block we just sent expected a reply (W=1 primary), record the
+  // outstanding system_bytes and arm T3.  Single-pending model — SECS-I
+  // is half-duplex on a serial link so multiple in-flight primaries are
+  // structurally rare.
+  if (!send_queue_.empty()) {
+    const auto& sent = send_queue_.front();
+    if (sent.header.w_bit) {
+      awaiting_reply_sys_ = sent.header.system_bytes;
+      out.push_back(ActionStartTimer{Timer::T3});
+    }
+  }
   send_queue_.pop_front();
   send_block_bytes_.clear();
   state_ = State::Idle;
@@ -52,6 +63,25 @@ void Protocol::deliver_recv(std::vector<Action>& out) {
     std::size_t consumed = 0;
     Block b = Block::decode(recv_buf_.data(), recv_buf_.size(), consumed);
     out.push_back(ActionTransmit{{kACK}});
+
+    // T3 reply correlation: if we were awaiting a reply with these
+    // system_bytes, cancel T3 — the transaction is closing.
+    if (awaiting_reply_sys_ && *awaiting_reply_sys_ == b.header.system_bytes) {
+      awaiting_reply_sys_.reset();
+      out.push_back(ActionCancelTimer{Timer::T3});
+    }
+
+    // T4 multi-block continuation: cancel any pending T4, then arm a
+    // fresh one if this block is NOT the final one (end_block=false).
+    if (awaiting_next_block_) {
+      awaiting_next_block_ = false;
+      out.push_back(ActionCancelTimer{Timer::T4});
+    }
+    if (!b.header.end_block) {
+      awaiting_next_block_ = true;
+      out.push_back(ActionStartTimer{Timer::T4});
+    }
+
     out.push_back(ActionDeliverBlock{std::move(b)});
   } catch (const BlockError& e) {
     out.push_back(ActionTransmit{{kNAK}});
@@ -67,12 +97,16 @@ void Protocol::deliver_recv(std::vector<Action>& out) {
 void Protocol::abort(std::string reason, std::vector<Action>& out) {
   out.push_back(ActionCancelTimer{Timer::T1});
   out.push_back(ActionCancelTimer{Timer::T2});
+  if (awaiting_reply_sys_)   out.push_back(ActionCancelTimer{Timer::T3});
+  if (awaiting_next_block_)  out.push_back(ActionCancelTimer{Timer::T4});
   out.push_back(ActionRaiseError{std::move(reason)});
   send_queue_.clear();
   send_block_bytes_.clear();
   recv_buf_.clear();
   recv_expected_ = 0;
   rty_ = 0;
+  awaiting_reply_sys_.reset();
+  awaiting_next_block_ = false;
   state_ = State::Idle;
 }
 
@@ -100,8 +134,21 @@ void Protocol::on_event(const Event& ev, std::vector<Action>& out) {
         if (state_ == State::RecvBlock) abort("T1 inter-character timeout", out);
         return;
       case Timer::T3:
+        // Reply timeout for an outstanding W=1 primary.  Abort with a
+        // diagnostic the host can route to its application-level retry
+        // / error path.
+        if (awaiting_reply_sys_) {
+          awaiting_reply_sys_.reset();
+          abort("T3 reply timeout", out);
+        }
+        return;
       case Timer::T4:
-        // Driven at the higher layer; FSM itself does not enforce.
+        // Inter-block gap exceeded; the peer never sent the next block
+        // of a multi-block message.
+        if (awaiting_next_block_) {
+          awaiting_next_block_ = false;
+          abort("T4 inter-block timeout", out);
+        }
         return;
     }
     return;

tests/test_secsi_timers.cpp

@@ -97,19 +97,161 @@ TEST_CASE("SECS-I T2: timeout in RecvBlock aborts (mid-block stall)") {
   CHECK(has_abort(out));
 }
 
-TEST_CASE("SECS-I T3 / T4: FSM-level no-ops (upper layer enforces)") {
+TEST_CASE("SECS-I T3: fires after a W=1 send is acked, expiry aborts") {
-  // The protocol docs in protocol.hpp note T3 (reply) and T4 (inter-block)
+  // Send a primary (w_bit=true), peer EOTs, peer ACKs the block.  The
-  // are driven from the higher SECS-II / message layer.  If a future
+  // FSM should now be awaiting a reply on those system_bytes.  Firing
-  // commit adds in-FSM handling for either, this test will break and
+  // T3 aborts the transaction.
-  // the documentation should be updated alongside.
+  Block primary;
+  primary.header.stream = 1;
+  primary.header.function = 1;
+  primary.header.w_bit = true;
+  primary.header.block_number = 1;
+  primary.header.end_block = true;
+  primary.header.system_bytes = 0xCAFEBABE;
+
   Protocol p(Role::Master);
   std::vector<Action> out;
+  p.on_event(EventSend{primary}, out);
+  out.clear();
+  p.on_event(EventByte{kEOT}, out);   // peer clears us
+  out.clear();
+  p.on_event(EventByte{kACK}, out);   // peer ACKs the block
+  // FSM should be back to Idle, awaiting a reply with T3 armed.
+  CHECK(p.state() == Protocol::State::Idle);
+  CHECK(p.awaiting_reply());
+  // The T3 ActionStartTimer should be present in `out`.
+  bool t3_armed = std::any_of(out.begin(), out.end(), [](const Action& a) {
+    auto* t = std::get_if<ActionStartTimer>(&a);
+    return t && t->which == Timer::T3;
+  });
+  CHECK(t3_armed);
+
+  // Now fire T3.
+  out.clear();
   p.on_event(EventTimeout{Timer::T3}, out);
   CHECK(p.state() == Protocol::State::Idle);
-  CHECK(out.empty());
+  CHECK_FALSE(p.awaiting_reply());
+  CHECK(has_abort(out));
+  bool t3_mentioned = std::any_of(out.begin(), out.end(), [](const Action& a) {
+    auto* err = std::get_if<ActionRaiseError>(&a);
+    return err && err->reason.find("T3") != std::string::npos;
+  });
+  CHECK(t3_mentioned);
+}
+
+TEST_CASE("SECS-I T3: cancels when matching reply arrives") {
+  // Same setup as above, but instead of T3 expiring we receive a reply
+  // block whose system_bytes match.
+  Block primary;
+  primary.header.stream = 1;
+  primary.header.function = 1;
+  primary.header.w_bit = true;
+  primary.header.end_block = true;
+  primary.header.block_number = 1;
+  primary.header.system_bytes = 0xCAFEBABE;
+
+  Protocol p(Role::Master);
+  std::vector<Action> out;
+  p.on_event(EventSend{primary}, out);
+  p.on_event(EventByte{kEOT}, out);
+  out.clear();
+  p.on_event(EventByte{kACK}, out);
+  REQUIRE(p.awaiting_reply());
+  out.clear();
+
+  // Peer sends ENQ for the reply.
+  p.on_event(EventByte{kENQ}, out);
+  out.clear();
+  // Then the reply block (header + body + checksum).
+  Block reply;
+  reply.header.stream = 1;
+  reply.header.function = 2;
+  reply.header.end_block = true;
+  reply.header.block_number = 1;
+  reply.header.system_bytes = 0xCAFEBABE;
+  auto reply_bytes = reply.encode();
+  for (auto byte : reply_bytes) {
+    p.on_event(EventByte{byte}, out);
+  }
+  // T3 should have been cancelled in deliver_recv.
+  CHECK_FALSE(p.awaiting_reply());
+  bool t3_cancelled = std::any_of(out.begin(), out.end(), [](const Action& a) {
+    auto* t = std::get_if<ActionCancelTimer>(&a);
+    return t && t->which == Timer::T3;
+  });
+  CHECK(t3_cancelled);
+}
+
+TEST_CASE("SECS-I T4: fires after a non-final block, expiry aborts") {
+  // Receive a block whose end_block bit is false (i.e. multi-block
+  // continuation expected); T4 should be armed, and firing it aborts.
+  Block mid;
+  mid.header.stream = 7;
+  mid.header.function = 3;
+  mid.header.block_number = 1;
+  mid.header.end_block = false;
+  mid.header.system_bytes = 0xDEAD;
+  auto bytes = mid.encode();
+
+  Protocol p(Role::Master);
+  std::vector<Action> out;
+  p.on_event(EventByte{kENQ}, out);
+  out.clear();
+  for (auto byte : bytes) {
+    p.on_event(EventByte{byte}, out);
+  }
+  CHECK(p.awaiting_next_block());
+  bool t4_armed = std::any_of(out.begin(), out.end(), [](const Action& a) {
+    auto* t = std::get_if<ActionStartTimer>(&a);
+    return t && t->which == Timer::T4;
+  });
+  CHECK(t4_armed);
+
+  out.clear();
   p.on_event(EventTimeout{Timer::T4}, out);
-  CHECK(p.state() == Protocol::State::Idle);
+  CHECK_FALSE(p.awaiting_next_block());
-  CHECK(out.empty());
+  CHECK(has_abort(out));
+  bool t4_mentioned = std::any_of(out.begin(), out.end(), [](const Action& a) {
+    auto* err = std::get_if<ActionRaiseError>(&a);
+    return err && err->reason.find("T4") != std::string::npos;
+  });
+  CHECK(t4_mentioned);
+}
+
+TEST_CASE("SECS-I T4: cancels when the next block arrives") {
+  Block mid;
+  mid.header.stream = 7;
+  mid.header.function = 3;
+  mid.header.block_number = 1;
+  mid.header.end_block = false;
+  mid.header.system_bytes = 0xDEAD;
+  auto bytes1 = mid.encode();
+
+  Block final_block;
+  final_block.header.stream = 7;
+  final_block.header.function = 3;
+  final_block.header.block_number = 2;
+  final_block.header.end_block = true;
+  final_block.header.system_bytes = 0xDEAD;
+  auto bytes2 = final_block.encode();
+
+  Protocol p(Role::Master);
+  std::vector<Action> out;
+  p.on_event(EventByte{kENQ}, out);
+  for (auto b : bytes1) p.on_event(EventByte{b}, out);
+  REQUIRE(p.awaiting_next_block());
+
+  // Second block.  Peer ENQs again (each block is its own send cycle).
+  out.clear();
+  p.on_event(EventByte{kENQ}, out);
+  out.clear();
+  for (auto b : bytes2) p.on_event(EventByte{b}, out);
+  CHECK_FALSE(p.awaiting_next_block());
+  bool t4_cancelled = std::any_of(out.begin(), out.end(), [](const Action& a) {
+    auto* t = std::get_if<ActionCancelTimer>(&a);
+    return t && t->which == Timer::T4;
+  });
+  CHECK(t4_cancelled);
 }
 
 TEST_CASE("SECS-I T2: in send state retries (existing) AND in recv state aborts (new)") {