Skip to content

Commit 62a364e

Browse files
committed
fix(slack): version continuation deletion checkpoints
1 parent 9a0c190 commit 62a364e

4 files changed

Lines changed: 237 additions & 27 deletions

File tree

crates/locality-slack/src/portable/hosted/checkpoint.rs

Lines changed: 180 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ use super::native::{
1717

1818
pub const HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V1: u16 = 1;
1919
pub const HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V1: u16 = 1;
20+
pub const HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V2: u16 = 2;
21+
pub const HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V2: u16 = 2;
2022
pub const MAX_HOSTED_SLACK_CURSOR_BYTES_V1: usize = 1024;
2123
pub const MAX_HOSTED_SLACK_APPLIED_PAGES_V1: usize = 256;
2224
pub const MAX_HOSTED_SLACK_REPLAY_BYTES_V1: usize = 512 * 1024;
@@ -137,6 +139,12 @@ struct HostedSlackPollCheckpointWireV1 {
137139
evidence: Vec<HostedSlackPollEvidenceV1>,
138140
}
139141

142+
#[derive(Deserialize)]
143+
struct HostedSlackPollCheckpointVersionHeader {
144+
checkpoint_format_version: u16,
145+
minimum_reader_version: u16,
146+
}
147+
140148
impl From<HostedSlackPollCheckpointWireV1> for HostedSlackPollCheckpointV1 {
141149
fn from(wire: HostedSlackPollCheckpointWireV1) -> Self {
142150
Self {
@@ -326,15 +334,12 @@ impl HostedSlackPollCheckpointV1 {
326334
}
327335

328336
pub(super) fn validate_internal(&self) -> Result<(), HostedSlackPollError> {
329-
if self.checkpoint_format_version != HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V1
330-
|| self.minimum_reader_version == 0
331-
|| self.minimum_reader_version > HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V1
332-
{
333-
return Err(HostedSlackPollError::UnsupportedVersion {
334-
format_version: self.checkpoint_format_version,
335-
minimum_reader_version: self.minimum_reader_version,
336-
});
337-
}
337+
validate_checkpoint_versions_for_reader(
338+
self.checkpoint_format_version,
339+
self.minimum_reader_version,
340+
HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V2,
341+
HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V2,
342+
)?;
338343
let selector = self.selector();
339344
selector
340345
.validate()
@@ -397,9 +402,6 @@ impl HostedSlackPollCheckpointV1 {
397402
self.backfill_cut_at.clone(),
398403
self.poll_overlap_watermark.clone(),
399404
);
400-
rebuilt.checkpoint_format_version = self.checkpoint_format_version;
401-
rebuilt.minimum_reader_version = self.minimum_reader_version;
402-
403405
for evidence in self.evidence.clone() {
404406
match evidence {
405407
HostedSlackPollEvidenceV1::AppliedPage { page } => {
@@ -453,6 +455,18 @@ impl HostedSlackPollCheckpointV1 {
453455

454456
pub fn decode_hosted_slack_poll_checkpoint_v1(
455457
bytes: &[u8],
458+
) -> Result<HostedSlackPollCheckpointV1, HostedSlackPollError> {
459+
decode_hosted_slack_poll_checkpoint_for_reader(
460+
bytes,
461+
HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V2,
462+
HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V2,
463+
)
464+
}
465+
466+
fn decode_hosted_slack_poll_checkpoint_for_reader(
467+
bytes: &[u8],
468+
supported_format_version: u16,
469+
supported_reader_version: u16,
456470
) -> Result<HostedSlackPollCheckpointV1, HostedSlackPollError> {
457471
if bytes.len() > MAX_HOSTED_SLACK_CHECKPOINT_BYTES_V1 {
458472
return Err(HostedSlackPollError::InputTooLarge {
@@ -461,6 +475,14 @@ pub fn decode_hosted_slack_poll_checkpoint_v1(
461475
actual_bytes: bytes.len(),
462476
});
463477
}
478+
let header = serde_json::from_slice::<HostedSlackPollCheckpointVersionHeader>(bytes)
479+
.map_err(|_| HostedSlackPollError::InvalidJson("checkpoint"))?;
480+
validate_checkpoint_versions_for_reader(
481+
header.checkpoint_format_version,
482+
header.minimum_reader_version,
483+
supported_format_version,
484+
supported_reader_version,
485+
)?;
464486
let checkpoint = serde_json::from_slice::<HostedSlackPollCheckpointV1>(bytes)
465487
.map_err(|_| HostedSlackPollError::InvalidJson("checkpoint"))?;
466488
checkpoint.validate()?;
@@ -479,6 +501,11 @@ pub enum HostedSlackPollError {
479501
format_version: u16,
480502
minimum_reader_version: u16,
481503
},
504+
ReaderUpdateRequired {
505+
format_version: u16,
506+
minimum_reader_version: u16,
507+
supported_reader_version: u16,
508+
},
482509
InvalidIdentity(&'static str),
483510
InvalidCanonicalUtcTimestamp(&'static str),
484511
InvalidCutOrder,
@@ -527,6 +554,14 @@ impl Display for HostedSlackPollError {
527554
formatter,
528555
"unsupported hosted Slack format {format_version} / reader {minimum_reader_version}"
529556
),
557+
Self::ReaderUpdateRequired {
558+
format_version,
559+
minimum_reader_version,
560+
supported_reader_version,
561+
} => write!(
562+
formatter,
563+
"hosted Slack reader update required for format {format_version} / reader {minimum_reader_version}; supported reader is {supported_reader_version}"
564+
),
530565
Self::InvalidIdentity(field) => write!(formatter, "invalid hosted Slack {field}"),
531566
Self::InvalidCanonicalUtcTimestamp(field) => {
532567
write!(formatter, "{field} must be canonical UTC seconds")
@@ -579,6 +614,39 @@ impl Display for HostedSlackPollError {
579614

580615
impl std::error::Error for HostedSlackPollError {}
581616

617+
pub(crate) fn validate_checkpoint_versions_for_reader(
618+
format_version: u16,
619+
minimum_reader_version: u16,
620+
supported_format_version: u16,
621+
supported_reader_version: u16,
622+
) -> Result<(), HostedSlackPollError> {
623+
if !matches!(
624+
(format_version, minimum_reader_version),
625+
(
626+
HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V1,
627+
HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V1
628+
) | (
629+
HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V2,
630+
HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V2
631+
)
632+
) {
633+
return Err(HostedSlackPollError::UnsupportedVersion {
634+
format_version,
635+
minimum_reader_version,
636+
});
637+
}
638+
if format_version > supported_format_version
639+
|| minimum_reader_version > supported_reader_version
640+
{
641+
return Err(HostedSlackPollError::ReaderUpdateRequired {
642+
format_version,
643+
minimum_reader_version,
644+
supported_reader_version,
645+
});
646+
}
647+
Ok(())
648+
}
649+
582650
pub(crate) fn parse_canonical_utc_timestamp(
583651
field: &'static str,
584652
value: &str,
@@ -970,3 +1038,103 @@ pub(crate) fn compare_slack_timestamps(left: &str, right: &str) -> Ordering {
9701038
.cmp(&right_seconds.len())
9711039
.then_with(|| left.cmp(right))
9721040
}
1041+
1042+
#[cfg(test)]
1043+
mod compatibility_tests {
1044+
use super::*;
1045+
use crate::portable::hosted::{
1046+
HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V2, HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V2,
1047+
HostedSlackRepliesPageV1,
1048+
};
1049+
1050+
const PRE_CHANGE_READER_GOLDEN: &[u8] =
1051+
include_bytes!("../../../fixtures/hosted-v1/poll-v1/checkpoint-reply-pagination-v1.json");
1052+
1053+
fn decode_with_pre_change_reader_semantics(
1054+
bytes: &[u8],
1055+
) -> Result<HostedSlackPollCheckpointV1, HostedSlackPollError> {
1056+
decode_hosted_slack_poll_checkpoint_for_reader(
1057+
bytes,
1058+
HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V1,
1059+
HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V1,
1060+
)
1061+
}
1062+
1063+
fn valid_v2_continuation_deletion_checkpoint() -> HostedSlackPollCheckpointV1 {
1064+
let mut checkpoint = decode_hosted_slack_poll_checkpoint_v1(PRE_CHANGE_READER_GOLDEN)
1065+
.expect("V1 continuation golden");
1066+
let root_message_id = checkpoint
1067+
.current_root_message_id
1068+
.clone()
1069+
.expect("continuation root");
1070+
let mut root = checkpoint
1071+
.candidate
1072+
.messages
1073+
.iter()
1074+
.find(|message| message.ts == root_message_id)
1075+
.cloned()
1076+
.expect("root message");
1077+
root.text.clear();
1078+
root.edited_ts = None;
1079+
root.deleted = true;
1080+
root.file_ids.clear();
1081+
let page = HostedSlackRepliesPageV1 {
1082+
page_format_version: HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V2,
1083+
minimum_reader_version: HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V2,
1084+
poll_kind: checkpoint.poll_kind,
1085+
phase: checkpoint.phase,
1086+
installation_id: checkpoint.installation_id.clone(),
1087+
team_id: checkpoint.team_id.clone(),
1088+
channel_id: checkpoint.channel_id.clone(),
1089+
sharing: checkpoint.sharing,
1090+
authorized_history_start_at: checkpoint.authorized_history_start_at.clone(),
1091+
backfill_cut_at: checkpoint.backfill_cut_at.clone(),
1092+
poll_cut_at: checkpoint.poll_cut_at.clone(),
1093+
poll_overlap_watermark: checkpoint.poll_overlap_watermark.clone(),
1094+
root_message_id,
1095+
root_reply_count: 0,
1096+
request_cursor: checkpoint.reply_cursor.clone(),
1097+
next_cursor: None,
1098+
observed_at: "2026-06-01T00:00:05Z".to_string(),
1099+
messages: vec![root],
1100+
users: Vec::new(),
1101+
files: Vec::new(),
1102+
};
1103+
checkpoint
1104+
.apply_replies_page(&page)
1105+
.expect("V2 continuation deletion");
1106+
checkpoint
1107+
}
1108+
1109+
#[test]
1110+
fn pre_change_reader_accepts_v1_golden_and_rejects_v2_before_replay() {
1111+
assert!(decode_with_pre_change_reader_semantics(PRE_CHANGE_READER_GOLDEN).is_ok());
1112+
1113+
let v2 = valid_v2_continuation_deletion_checkpoint();
1114+
let v2_bytes = serde_json::to_vec(&v2).unwrap();
1115+
assert_eq!(decode_hosted_slack_poll_checkpoint_v1(&v2_bytes), Ok(v2));
1116+
assert_eq!(
1117+
decode_with_pre_change_reader_semantics(&v2_bytes),
1118+
Err(HostedSlackPollError::ReaderUpdateRequired {
1119+
format_version: HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V2,
1120+
minimum_reader_version: HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V2,
1121+
supported_reader_version: HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V1,
1122+
})
1123+
);
1124+
1125+
let mut malformed_after_header =
1126+
serde_json::from_slice::<serde_json::Value>(&v2_bytes).unwrap();
1127+
malformed_after_header["evidence"][0]["page"]["canonical_page_json"] =
1128+
serde_json::json!("would fail if replayed");
1129+
assert_eq!(
1130+
decode_with_pre_change_reader_semantics(
1131+
&serde_json::to_vec(&malformed_after_header).unwrap()
1132+
),
1133+
Err(HostedSlackPollError::ReaderUpdateRequired {
1134+
format_version: HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V2,
1135+
minimum_reader_version: HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V2,
1136+
supported_reader_version: HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V1,
1137+
})
1138+
);
1139+
}
1140+
}

crates/locality-slack/src/portable/hosted/poll.rs

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use locality_protocol::{
77
use serde::{Deserialize, Serialize};
88

99
use super::checkpoint::{
10+
HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V2, HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V2,
1011
HostedSlackAppliedPageV1, HostedSlackCompletedRootV1, HostedSlackPollCheckpointV1,
1112
HostedSlackPollError, HostedSlackPollEvidenceV1, HostedSlackPollKindV1, HostedSlackPollPhaseV1,
1213
HostedSlackRootExpectationV1, MAX_HOSTED_SLACK_APPLIED_PAGES_V1, compare_slack_timestamps,
@@ -23,6 +24,8 @@ use super::render::{
2324

2425
pub const HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V1: u16 = 1;
2526
pub const HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V1: u16 = 1;
27+
pub const HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V2: u16 = 2;
28+
pub const HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V2: u16 = 2;
2629
pub const MAX_HOSTED_SLACK_POLL_PAGE_BYTES_V1: usize = 512 * 1024;
2730
pub const MAX_HOSTED_SLACK_POLL_PAGE_MESSAGES_V1: usize = 512;
2831
pub const MAX_HOSTED_SLACK_POLL_PAGE_USERS_V1: usize = 512;
@@ -125,6 +128,12 @@ pub fn hosted_slack_replies_page_reference_closure_v1(
125128
impl HostedSlackHistoryPageV1 {
126129
pub fn validate(&self) -> Result<(), HostedSlackPollError> {
127130
validate_page_versions(self.page_format_version, self.minimum_reader_version)?;
131+
if self.page_format_version != HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V1 {
132+
return Err(HostedSlackPollError::UnsupportedVersion {
133+
format_version: self.page_format_version,
134+
minimum_reader_version: self.minimum_reader_version,
135+
});
136+
}
128137
if !matches!(
129138
self.phase,
130139
HostedSlackPollPhaseV1::HistoricalHistory | HostedSlackPollPhaseV1::CatchUpHistory
@@ -247,6 +256,13 @@ impl HostedSlackRepliesPageV1 {
247256
));
248257
}
249258
let deleted_root_reconciliation = is_deleted_root_reconciliation_page(self);
259+
if self.page_format_version == HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V2
260+
&& !deleted_root_reconciliation
261+
{
262+
return Err(HostedSlackPollError::IncompleteCandidate(
263+
"V2 replies deletion reconciliation",
264+
));
265+
}
250266
if self.request_cursor.is_none() {
251267
let first = &self.messages[0];
252268
if first.ts != self.root_message_id {
@@ -454,6 +470,10 @@ impl HostedSlackPollCheckpointV1 {
454470
}
455471
let first_page = page.request_cursor.is_none();
456472
let deleted_root_reconciliation = is_deleted_root_reconciliation_page(page);
473+
if deleted_root_reconciliation {
474+
next.checkpoint_format_version = HOSTED_SLACK_POLL_CHECKPOINT_FORMAT_VERSION_V2;
475+
next.minimum_reader_version = HOSTED_SLACK_POLL_MINIMUM_READER_VERSION_V2;
476+
}
457477
let expectation_is_from_this_catch_up = page.phase
458478
!= HostedSlackPollPhaseV1::CatchUpReplies
459479
|| catch_up_history_contains_root(&next, &page.root_message_id)?
@@ -650,10 +670,16 @@ fn validate_page_versions(
650670
format_version: u16,
651671
minimum_reader_version: u16,
652672
) -> Result<(), HostedSlackPollError> {
653-
if format_version != HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V1
654-
|| minimum_reader_version == 0
655-
|| minimum_reader_version > HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V1
656-
{
673+
if !matches!(
674+
(format_version, minimum_reader_version),
675+
(
676+
HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V1,
677+
HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V1
678+
) | (
679+
HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V2,
680+
HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V2
681+
)
682+
) {
657683
return Err(HostedSlackPollError::UnsupportedVersion {
658684
format_version,
659685
minimum_reader_version,
@@ -1624,11 +1650,13 @@ fn normalized_root_id(message: &RawHostedSlackMessage) -> Option<&str> {
16241650
.filter(|thread_ts| *thread_ts != message.ts)
16251651
}
16261652

1627-
// V1 reconciliation deliberately uses only the existing replies-page schema so
1628-
// deny-unknown V1 readers never receive an unversioned field. The exact terminal
1629-
// tombstone shape is reserved for a root Slack reports as thread_not_found.
1653+
// V2 deliberately adds no fields to the deny-unknown V1 schema. Its explicit
1654+
// format/minimum-reader pair reserves this terminal tombstone shape for a root
1655+
// Slack reports as thread_not_found, including after continuation pages.
16301656
fn is_deleted_root_reconciliation_page(page: &HostedSlackRepliesPageV1) -> bool {
1631-
page.next_cursor.is_none()
1657+
page.page_format_version == HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V2
1658+
&& page.minimum_reader_version == HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V2
1659+
&& page.next_cursor.is_none()
16321660
&& page.root_reply_count == 0
16331661
&& page.messages.len() == 1
16341662
&& page.messages[0].ts == page.root_message_id

crates/locality-slack/src/portable/hosted/provider.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,11 @@ use super::native::{
2828
RawHostedSlackMessage, RawHostedSlackUser,
2929
};
3030
use super::poll::{
31-
HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V1, HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V1,
32-
HostedSlackHistoryMessageV1, HostedSlackHistoryPageV1, HostedSlackPollOutputV1,
33-
HostedSlackRepliesPageV1, hosted_slack_history_page_reference_closure_v1,
34-
hosted_slack_replies_page_reference_closure_v1,
31+
HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V1, HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V2,
32+
HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V1,
33+
HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V2, HostedSlackHistoryMessageV1,
34+
HostedSlackHistoryPageV1, HostedSlackPollOutputV1, HostedSlackRepliesPageV1,
35+
hosted_slack_history_page_reference_closure_v1, hosted_slack_replies_page_reference_closure_v1,
3536
};
3637

3738
pub const HOSTED_SLACK_PROVIDER_PAGE_LIMIT_V1: u32 = 15;
@@ -1009,8 +1010,8 @@ fn deleted_root_reconciliation_page(
10091010
root.file_ids.clear();
10101011
let selector = checkpoint.selector();
10111012
let page = HostedSlackRepliesPageV1 {
1012-
page_format_version: HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V1,
1013-
minimum_reader_version: HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V1,
1013+
page_format_version: HOSTED_SLACK_POLL_PAGE_FORMAT_VERSION_V2,
1014+
minimum_reader_version: HOSTED_SLACK_POLL_PAGE_MINIMUM_READER_VERSION_V2,
10141015
poll_kind: checkpoint.poll_kind(),
10151016
phase: request.phase,
10161017
installation_id: selector.installation_id,

0 commit comments

Comments
 (0)