feat(bluewave): export Blue Wave offline mail packets - #798
Conversation
|
This PR is now dependent on #799 |
|
Nice! I'll start a review in a bit, ty! |
So... this is definitely dependent on #799 ... When that's merged, I'll fill the gap in this to use the lifted module. |
NuSkooler
left a comment
There was a problem hiding this comment.
I went through the packet format against the Cutting Edge structure kit rather than taking the offsets on faith, and it holds up: INF_HEADER 1230, INF_AREA_INFO 80, MIX_REC 14, FTI_REC 186, and every field you write lands on the correct byte. The AreaFlags/MessageFlags values all match the kit's INF_* / FTI_MSG* defines, ver = 3 is right (PACKET_LEVEL 3, and MultiMail gates oldstyle on ver < 3), and the from_to_len/subject_len you advertise line up with writeFixed reserving the NUL. The hard part is correct.
Comments inline. The msglength one is the interesting one, and it's a request for a comment rather than a change.
Two things too small for their own threads: areanum[6] only holds 5 digits since writeFixed reserves the NUL, so a pinned 6-digit area number would truncate (auto-numbering starts at 1, so it never bites in practice); and the init() comment says a datStream write failure is "unhandled ... thrown at the process" directly above the line that handles it.
| // short of what an ENiGMA½ message ID can reach | ||
| number: (this.messageNumber += 1) & 0xffff, | ||
| offset: this.datOffset, | ||
| length: text.length + 1, |
There was a problem hiding this comment.
Worth recording why this is +1, because the two readers disagree and the kit is ambiguous.
The kit says "seek to this exact offset, then read msglength bytes ... to load the entire message text", and also that the leading space is "NOT to be considered part of the message". Those pull opposite ways, and implementations split on it:
BlueMail, bwave::getBody:
if (c == 0 && kar == ' ') if (++c < msglen) kar = fgetc(datFile);
-- the space counts.
MultiMail, bluewave::getblk:
if (!count && (kar == ' ')) kar = fgetc(infile);
-- no increment, the space is free.
So +1 is exactly right for BlueMail, and makes MultiMail read one byte past the text: it picks up the next message's leading space (or EOF, which it maps to ' '), i.e. one invisible trailing space. Going the other way would be exactly right for MultiMail and would drop the last character of every message in BlueMail. You picked the safer side and I'd keep it.
The reason to say so in the comment above: NoCarrierMail inherits MultiMail's version of this loop verbatim, so that's the reader where you'd notice the stray space -- and "fix" it straight into truncating every message for BlueMail users.
| // header of November 30 1995. Offsets are from the structure kit; see | ||
| // https://www.moon-soft.com/program/FORMAT/internet/bluewave.htm | ||
| // | ||
| const Inf = { |
There was a problem hiding this comment.
These constants are the same numbers the writer uses, so the suite is checking the writer against itself: if an offset were wrong, every test in this file would still pass. Worth being clear-eyed about what that buys -- it locks in round-trip consistency and would catch a refactor that shifted a field, but nothing here would catch a struct that disagreed with the kit.
I checked the layout by hand against bluewave.h and it is correct, so there's nothing to fix. But since you maintain NoCarrierMail: did you open a generated packet in it? Saying so in the PR description is worth more than any assertion in this file, and a checked-in fixture packet with a "these exact bytes" test would keep it true.
| 'messageNetworks.qwk.bbsID': { type: 'string' }, | ||
|
|
||
| 'messageNetworks.bluewave': { type: 'object', closedKeys: true }, | ||
| 'messageNetworks.bluewave.areas': { openMap: true }, |
There was a problem hiding this comment.
openMap frees the area tags, which is right, but it leaves the values unchecked -- and this PR introduces three of them (number, echotag, title). A mistyped echotag silently falls back to the derived tag, which changes what replies route by, and config validate won't say a word.
meta.js already handles this shape -- messageConferences.*.areas on line 255 uses the wildcard. Something like 'messageNetworks.bluewave.areas.*': { type: 'object', closedKeys: true } plus the three keys would close it.
messageNetworks.qwk.areas has the same hole with its conference key, so this is inherited rather than something you introduced -- but Blue Wave is where it starts to matter.
NuSkooler
left a comment
There was a problem hiding this comment.
One follow-up on the area flags: ENiGMA does already have the facility for this, so it's more actionable than I first thought.
| rec.writeUInt16LE( | ||
| AreaFlags.Scanning | | ||
| AreaFlags.Post | | ||
| (entry.area ? AreaFlags.Echo : AreaFlags.NoPublic), | ||
| 77 | ||
| ); | ||
| rec.writeUInt8(0, 79); // network_type: FidoNet |
There was a problem hiding this comment.
ENiGMA can tell these apart -- getMessageAreaByTag(areaTag).addressFlavor (Message.AddressFlavor: local | ftn | email | qwk | nntp | activitypub), already used this way in fse.js and new_scan.js.
That matters because the kit publishes an exact chart for how these two flags pair with NETWORK_TYPE:
INF_NET_FIDONET 0 Local = ECHO=off, NETMAIL=off
EchoMail = ECHO=on, NETMAIL=off
NetMail = ECHO=on, NETMAIL=on
INF_NET_INTERNET 1 Local = ECHO=off, NETMAIL=off
Newsgroup = ECHO=on, NETMAIL=off
E-mail = ECHO=on, NETMAIL=on
Against that, entry.area ? Echo : NoPublic with a hardcoded network_type of 0 misses three ways: a purely local base (the common case) gets INF_ECHO, which means network mail; the private area gets neither ECHO nor NETMAIL, so it reads as Local rather than NetMail; and an email/nntp/activitypub area is announced as FidoNet.
network_type isn't cosmetic, which I'd assumed at first -- MultiMail branches on INF_NET_INTERNET in six places: it stops stripping soft CRs, pulls \001From:, \001Message-ID: and \001References: out of the body in endproc, and switches reply addressing between internet and FidoNet.
None of it bites while the packet is one-way, but it's the field that decides reply addressing, so it's worth getting right before .UPL import lands.
@NuSkooler asked whether a generated packet had been opened in NoCarrierMail. It has now, and this is what it showed. The packet was built by driving In the readerIt opens. CP437 renders: box drawing, the Below the readerTwo things the screen cannot show, read out of the packet directly. The private message carries the flag. Its
The area records:
The generator is a throwaway script rather than a checked-in fixture, so it does not answer the other half of that thread — a fixture packet with a byte-exact test would still be the thing that keeps the layout honest over time. |
Refs NuSkooler#119 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The branch is rebased onto master now that #799 has merged, so the diff here is Blue Wave alone. The caveat on my previous comment is spent: the Six commits sit on top of the original. Answering the reviewArea kinds from
The closed key set. One correction worth passing on: the wildcard form does not fire. I wrote The fixture. Also in: a warning when a pinned area number exceeds five digits, since Beyond the reviewThe export is now reachable. Nothing in This is scope andy5995 and I agreed to add rather than something the review asked for, and it leaves QWK as the one offline format with no way in. Reverting it is Still not done: reply packets, so |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Refs #119
Are you interested in this? I can add import to it...