fix: encode MODSEQ message data item with parentheses

[marshallr12]

Fixes #722.

The defect

RFC 7162 §7 defines the FETCH response data item as

fetch-mod-resp     = "MODSEQ" SP "(" permsg-modsequence ")"
msg-att-dynamic    =/ fetch-mod-resp

but MessageDataItem::ModSeq encoded as MODSEQ 624140003 — without the parentheses. The decoder (msg_att_dynamic) already required them, so the two disagreed and a response carrying this data item did not survive a decode → encode → decode round trip:

encoded: "* 1 FETCH (MODSEQ 624140003)\r\n"
re-decode ok? false

Note the asymmetry that likely caused this: the command data item name is the bare MODSEQ, and only the response data item is parenthesized. MessageDataItemName::ModSeq was already correct and is unchanged here.

The change

• imap-codec/src/codec/encode.rs — one line, MODSEQ {value} → MODSEQ ({value}), with a note recording the command/response asymmetry so it doesn't get "simplified" back.
• imap-codec/src/fetch.rs — two regression tests, both gated on ext_condstore_qresync and both failing before the fix:
  • test_encode_message_data_item_modseq — known-answer encode of MODSEQ (624140003).
  • test_kat_inverse_response_fetch_modseq — kat_inverse_response over all three FETCH responses from RFC 7162 §3.1.4.2, Example 13.
• Also added the fetch-mod-resp production to msg_att_dynamic's ABNF doc comment, which previously only documented FLAGS.

No API change, so no SemVer impact.

Verification

just isn't installed here, so I ran the CI legs directly, all with RUSTFLAGS/RUSTDOCFLAGS = -D warnings:

• cargo +nightly fmt --check
• cargo clippy --workspace --all-targets --exclude imap-codec-bench, and again with --all-features
• cargo test --workspace --exclude imap-types-fuzz --exclude imap-codec-fuzz --all-targets --exclude imap-codec-bench, and again with --all-features
• cargo test -p imap-codec --no-default-features --features ext_condstore_qresync (the feature in isolation)
• cargo doc --no-deps --document-private-items --keep-going --all-features

All green. Reverting just the encoder line turns both new tests red, so they do fence the bug.

🤖 Generated with Claude Code

https://claude.ai/code/session_012LQkXHvJk9Ls8v9odBhonf

[duesee]

Thank you! I left some comments inline and also have a more general request.

I feel that most comments are redundant given that we include the ABNF definition. They are not wrong but IMAP made a few... let's say... "questionable" syntax choices so what people generally should do is to ignore all prose and stick to the ABNF anyway. So I feel there is no need to point out these curiosities.

Also, I have not decided yet how to handle AI contributions and this would be the first one (co-authored: claude). Given this PR merely adjusts a string (by adding ( and )) and adds some (straightforward) tests, I would ask to "redo" the PR without AI. This way I could merge it w/o having to devide how I feel about AI contributions just yet.

[coveralls]

Coverage Report for CI Build 32047805405

Coverage increased (+0.02%) to 91.385%

Details

• Coverage increased (+0.02%) from the base build.
• Patch coverage: 19 of 19 lines across 2 files are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	11619
Covered Lines:	10618
Line Coverage:	91.38%
Coverage Strength:	915.3 hits per line

---

💛 - Coveralls

[duesee]

Thanks! I think we can almost merge :-) One remaining question: Can you say a bit more about the tests? Do we need 3 cases? How are they different? Maybe I am missing something.

[soywod]

No we don't really need, I just thought it could be nice to add examples directly from RFC. It can be trimmed down.

[duesee]

I'm confused now :D Comment in wrong PR?

[soywod]

Oups, it was not my PR, somehow it looked like. I need to sleep. Sorry 😄

[marshallr12]

Thanks! I think we can almost merge :-) One remaining question: Can you say a bit more about the tests? Do we need 3 cases? How are they different? Maybe I am missing something.

The three cases inside the round-trip test are just RFC 7162 §3.1.4.2 Example 13 kept whole. They're structurally identical and any one of them catches the regression, so they're there for readability against the spec
rather than for coverage. Given your point about sticking to the ABNF and ignoring the prose, I suspect you'd rather have one — say the word and I'll collapse them.

[duesee]

I see! Thanks for the clarification. I think in this particular case I would indeed prefer to collapse them.

[duesee]

Thank you very much!