Skip to content

Commit 42e8ecd

Browse files
authored
fix: handle FDv2 intentCode none as listening, reject unknown codes (#555)
Bring the FDv2 protocol handler's `server-intent` dispatch in line with the spec and the Java implementation. - `intentCode: none` keeps the handler in listening state so a subsequent partial cycle works. - Unrecognised intent codes return a protocol error instead of being collapsed with `none`. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes streaming protocol state machine behavior for `server-intent`, which can affect how clients process FDv2 payloads; scope is limited to intent dispatch with new tests. > > **Overview** > Aligns **`server-intent`** handling in `FDv2ProtocolHandler` with the FDv2 spec and Java client. > > For **`intentCode: none`**, the handler still emits an immediate empty **`kNone`** changeset, but it now leaves internal state in **partial/listening** mode instead of **inactive**, so a later **`put-object`** / **`payload-transferred`** cycle can produce a partial changeset. > > **Unrecognized** intent codes (deserialized as **`kUnknown`**) are no longer treated like **`none`**: the handler **resets** and returns a **`ProtocolError`** with *"server-intent had an unrecognized intent code"*. > > Tests cover a partial cycle after **`none`** and protocol errors for unknown codes (e.g. **`future-code`**). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f40b13f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent d6368b8 commit 42e8ecd

2 files changed

Lines changed: 38 additions & 3 deletions

File tree

libs/internal/src/fdv2_protocol_handler.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,15 @@ FDv2ProtocolHandler::Result FDv2ProtocolHandler::HandleServerIntent(
7373
state_ = State::kFull;
7474
} else if (code == IntentCode::kTransferChanges) {
7575
state_ = State::kPartial;
76-
} else {
77-
// kNone or kUnknown: emit an empty changeset immediately.
78-
state_ = State::kInactive;
76+
} else if (code == IntentCode::kNone) {
77+
state_ = State::kPartial;
7978
return data_model::FDv2ChangeSet{
8079
data_model::ChangeSetType::kNone, {}, data_model::Selector{}};
80+
} else {
81+
// kUnknown: an intent code we don't recognise.
82+
Reset();
83+
return Error::ProtocolError(
84+
"server-intent had an unrecognized intent code");
8185
}
8286
return std::monostate{};
8387
}

libs/internal/tests/fdv2_protocol_handler_test.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,37 @@ TEST(FDv2ProtocolHandlerTest, NoneIntentEmitsEmptyChangeSetImmediately) {
6262
EXPECT_FALSE(cs->selector.value.has_value());
6363
}
6464

65+
TEST(FDv2ProtocolHandlerTest, NoneIntentAllowsSubsequentPartialCycle) {
66+
FDv2ProtocolHandler handler;
67+
68+
handler.HandleEvent("server-intent", MakeServerIntent("none"));
69+
handler.HandleEvent("put-object",
70+
MakePutObject("flag", "my-flag", kFlagJson));
71+
auto result = handler.HandleEvent("payload-transferred",
72+
MakePayloadTransferred("s1", 1));
73+
74+
auto* cs = std::get_if<data_model::FDv2ChangeSet>(&result);
75+
ASSERT_NE(cs, nullptr);
76+
EXPECT_EQ(cs->type, data_model::ChangeSetType::kPartial);
77+
ASSERT_EQ(cs->changes.size(), 1u);
78+
EXPECT_EQ(cs->changes[0].key, "my-flag");
79+
}
80+
81+
// ============================================================================
82+
// Unknown intent
83+
// ============================================================================
84+
85+
TEST(FDv2ProtocolHandlerTest, UnknownIntentReturnsProtocolError) {
86+
FDv2ProtocolHandler handler;
87+
88+
auto result =
89+
handler.HandleEvent("server-intent", MakeServerIntent("future-code"));
90+
91+
auto* err = std::get_if<FDv2ProtocolHandler::Error>(&result);
92+
ASSERT_NE(err, nullptr);
93+
EXPECT_EQ(err->kind, FDv2ProtocolHandler::Error::Kind::kProtocolError);
94+
}
95+
6596
// ============================================================================
6697
// kTransferFull intent
6798
// ============================================================================

0 commit comments

Comments
 (0)