75 KiB
Round 3: drafts E and F
Draft E, junior seat
-
Hardest places, hardest first
src/session-life.ts:175SessionLife.attempt(), withenter()at:115andlinkLost()at:109. The async reconnect continuation re-enters the state machine after two awaits. It snapshotsthis.linksand later compares it, and it reads state throughis()only to stop TypeScript narrowing. The step toboundis not in this file: it goesrebind→client.tsbind()→Session.bound()→life.bound(), three files away. There is also hidden re-entrancy.effects.linkDown()callssock.destroy(), which fires the transport'sonClose→life.linkLost(). That call is harmless only becauseattached()is already false. The ASCII diagram at:8-24and the comment "a transition from any other state is ignored" resolved most of it. Whylinkscould change duringrebindstayed opaque.src/outgoing-requests.ts:97request(),sendOnce()at:116andattempt()at:162. There is afor(;;)retry around three nested waits: link, window slot, response. Each has its own deadline or timeout semantics, andwrittenplusstate() !== 'down'decide whether to loop. Themisuse()check runs here and again inSession.send()(session.ts:180), and the reason for the duplicate is a riddle comment ("named as one ahead of the drain"). The JSDoc onrequest()and theUnansweredErrornaming resolved the intent. I had to readREADME"Sends and the link" to trust it.src/handled-messages.ts:61refuses()and:120run().refuses()looks like a predicate but sweeps, logs, and flipsatBoundhysteresis.run()answers the peer after the handler, and the answer depends on whethersms.answeredwas flipped by a closure insidesms.ts. It then removes the entry only ifrunning.get(key) === sms, because a sweep may already have dropped it while the handler keeps running.ExpiringGroupsgetsmaxhere but, by its own doc, does not enforce it, so I had to go and readexpiring-groups.tsto know who does.src/reassembly.ts:186trim(), withExpiringGroups.weigh()atsrc/expiring-groups.ts:70.weigh()evicts the oldest groups and may return the current key itself. Thenanswered = parts.size - 1subtracts the just-arrived segment, because that one gets afullrefusal and the peer keeps it. Holding "set never evicts, weigh does, owners check full" across two files cost two rereads. The inline comments resolved it.src/sms.ts:82createSms()and:126sendResp(). A mutableanswerrecord is captured in a closure and exposed through getters.linkis the socket at arrival, compared bydestroyedrather than againstsession.sock.answeredAsmeans "multipart, already answered on arrival". I only understood whysendResp()on a multipart message is a no-op after reading README "Server in depth" (answered on arrival). The code alone did not tell me.src/pdu.ts:84resolveShortMessage()and:113resolveBody(). TheCodingSourceidea is hard for someone without SMPP: which ofshort_messageandmessage_payloadgets to setdata_coding, and when an empty buffer counts as "payload". Also,readOptionalParams()at:267retries parsing with one skipped NULL. The comments are accurate but assume the domain. README "Building" and the SMPP terms table were needed.src/defs/encodings.ts:152-191messageClassOf(),messageClassEncoding(),encodingByDataCoding(). Bit masks (0x80,0x10,0xF0,>> 2 & 0x03) against GSM 03.38 coding groups I have never seen. The comments cite spec sections I cannot open. This stayed opaque. I trust it only because the tests presumably pin it; I did not open them.src/client.ts:240retryUntilBound()and:278keepTrying(). This is a second backoff loop, separate fromSessionLife's. Itssettlecallback is called on every failure and does not settle anything; it only recordslastErr. The name misled me until I read the callback body at:290. AGENTS' architecture line ("the first-connect retry of reconnect.fromStart") told me why it exists.
-
The unit I would least want to modify:
SessionLife.enter()/attempt()(src/session-life.ts:115-203). Every lifecycle effect fans out from it throughLifeEffectsclosures defined insession.ts:260. Those closures call back into the transport, whose socket events calllife.linkLost()again. Correctness rests on "ignored from any other state" and on the ordering of effects before emit. I could not predict what a new transition would re-trigger without running it. -
Expected hard, found easy:
PduFramer: small, one job, and the quadratic-avoidance comment explains the only trick.- The wire-type table in
defs/commands.ts, where the wire-order warning sits right at the table. - The result pattern and "nothing throws": consistent everywhere, so no surprises.
defaults.ts: one place, grouped.sms-id.ts,concat.ts,message-body.ts,log.ts,pdu-refusal.ts: each read cold in one pass.send-sms.ts: long but linear, a chain ofcheck*functions.- The AGENTS architecture map, which got me to the right file first time for every question I had.
-
Prose debt
- Documentation I needed:
- README "SMPP terms" table: essential and cheap to find, linked from the table of contents.
- README "Server in depth" and "Receiving in depth", for answered-on-arrival and the handled-message bound. The rules behind
sms.tsandhandled-messages.tslive there, not in the code. - AGENTS architecture list: essential for navigation.
- The AGENTS decisions index lines are cryptic without
docs/decisions.md, which I was not allowed to open (for example "A report is final unless itsesm_classor its state says otherwise"). - Spec knowledge (
esm_classbits,data_codinggroups) is cited by section number and never explained. That cost the most and was never repaid.
- Prose that told me nothing the code did not already say:
- The duplicated deliver JSDoc in
outgoing-requests.ts:84andpending-requests.ts:57. /** A socket is on the link. */onattached().- The AGENTS "Defects found in 0.4.0" table, which is history and not a guide to the current code.
- The AGENTS test conventions, which are irrelevant to reading
src/. - A different cost: many comments are compressed to riddles and needed several reads each. Examples are "Announced when the wait is over rather than when it starts: a cancelled one never happened." (
session-life.ts:160) and "A misuse is named as one ahead of the drain, rather than blamed on the shutdown."
- The duplicated deliver JSDoc in
- Documentation I needed:
-
Scores
- Navigation 7: at the "Predictable" anchor. The AGENTS file map and honest file names (
pdu-framer,link-timers,sms-id) took me symptom → file first try. It stops short of 9 because reconnect lives in two places (session-life.tsand the separateclient.tsretry loop). - Locality 5: at the "Honest middle" anchor. Collaborators are wired by closures back into
Session(LifeEffects,PduTransportcallbacks,IncomingRequestscallingsession.emit,.sockand.close). TheSmsanswer record is mutated from two modules. Safe changes inSessionLifeandHandledMessagesneed the whole session held in your head. - Shape 6: between 5 and 7. Fan-out per level is bounded and the files are small. Names overload or lie, though:
settlemeans four different things (IdleWaiters,PendingRequests,SendWindow, theclient.tscallback that settles nothing).handlersinIncomingRequestsmeans{answer, send}, notonSms.refuses()andclosing()read as predicates but mutate.SessionLife.carries()is dead: unused insrc/, duplicated by the switch inOutgoingRequests.waitForLink().
- Self-sufficiency 5: at the "Honest middle" anchor. The generic parts (framer, codec types, results, timers) stand alone. The session semantics (answered-on-arrival, the handled-message bound) and all the
data_coding/esm_classbit logic needed README sections or the spec open beside the code. - Overall 5: as someone new to the domain, I would take an area in a day or two, with rereads and some wrong turns. The problem's intrinsic difficulty is genuinely high (a protocol state machine, reassembly, and receipt correlation over a flaky link), and it gets no bonus here.
- Navigation 7: at the "Predictable" anchor. The AGENTS file map and honest file names (
SCORES nav=7 loc=5 shape=6 self=5 overall=5
Draft E, mid seat
-
Hardest places, hardest first
- The answer to an inbound message,
incoming-requests.ts:218(IncomingRequests.onMessage),handled-messages.ts:120(HandledMessages.run) andsms.ts:126(sendResp), withsms.ts:82(createSms) holding theAnswerrecord.- Whether the peer has been answered, and under which id, is decided in three places:
onMessageanswers multipart segments one by one on arrival and passesansweredAs.createSmsturnsansweredAsinto a pre-setstatus: 'ESME_ROK'.runanswers after the handler only when!sms.answered, choosing the retry status if the handler failed.
- "Which link" is carried as a
Socketidentity that is compared through.destroyedin two places (IncomingRequests.handleandsendResp). - To reason about "handler throws after a multipart message" I had to keep all three files in my head. The
Smstype docs and the README section "Server in depth" resolved it. It is followable but not local.
- Whether the peer has been answered, and under which id, is decided in three places:
ExpiringGroupsand the classes built on it,expiring-groups.ts:19, withreassembly.ts:187(Reassembler.trim) anddlr-merger.ts:105/150/166(collect,open,spend).- The store refuses to enforce its own
maxandtimeout("owners check full and call takeExpired(); only weigh() evicts"). So each of the three owners re-implements the capping itself (open→dropOldest,sweepbeforecollect). weigh()can evict the very key being weighed.trimthen doesanswered = parts.size - 1for that case, and I needed two reads to see why.DlrMergerruns a secondExpiringGroups<true>(spent) as a tombstone set, withspend()writing to both stores.- The doc comments stop you misusing the store, but understanding any one owner means holding the store's partial contract.
- The store refuses to enforce its own
resolveShortMessage/resolveBody,pdu.ts:84andpdu.ts:113.CodingSourcedecides whethershort_messageormessage_payloadmay setdata_coding. That depends on Buffer vs string vs empty, and on whether the command's table has ashort_messageat all.- I had to hold four or five branches at once, and
data_codinggets rewritten in two different spots. - The type comment on
CodingSourcehelped. The rule behind it ("a string body is written in the alphabet its own data_coding names") is only a title in the AGENTS index.
- The retry loop in
OutgoingRequests.request/sendOnce,outgoing-requests.ts:97and:116.sendOncereturnsundefinedto mean "loop again". Whether to loop depends onattempt.writtenand on a live read ofstate() !== 'down'after two awaits.- With
LinkWaiters,SendWindowandPendingRequestsunderneath, a send passes through four waits with three different abort and timeout rules. - The doc comments on
requestandAttempt.writtenresolved it, as did the README bullets under "Sends and the link".
SessionLife.enter/attempt,session-life.ts:115and:175.- The ASCII state diagram is very good. What cost me was
attempt(): it re-checksis('down')afterconnect, thenthis.links !== link || !is('connected')afterrebind. rebindcallsSession.bound(), which callslife.bound()→enter('bound'). That is a re-entrant path back into the machine from inside an await.- The comment "the one continuation that re-enters the machine" marks it, and the diagram resolved it.
- The ASCII state diagram is very good. What cost me was
Session.unbind/drain,session.ts:218and:317.life.closing()is a query-named method that performs the transition.closedOnUnbind = wasOpen && !this.life.attached()infers that the peer dropped the link in answer to our unbind.- The return value orders two errors with different priorities.
- The doc comment helped. The mutating
closing()still surprised me.
- The two reconnect loops,
client.ts:240(retryUntilBound) andclient.ts:278(keepTrying).- AGENTS says "the reconnect loop is the
downstate", butfromStartis a second, hand-rolled backoff loop inclient.ts. The charter's file list does mention it. - The callback named
settleis called on every failed attempt and does not settle: it only recordslastErr. That name misled me until I read the body ofkeepTrying.
- AGENTS says "the reconnect loop is the
- The overloaded third argument of
WireType.read,pdu.ts:249(readParamspassessm_lengthto every param reader) anddefs/types.ts:280(tlvInt).- For a mandatory parameter it is
sm_length; for a TLV it is the TLV header length. - Only
bufferand the tlv variants use it, and nothing names the dual meaning. I found it by grepping callers. It stayed half-opaque until then.
- For a mandatory parameter it is
- The answer to an inbound message,
-
The unit I would least want to modify:
Reassembler.collect+trimtogether withExpiringGroups.weigh.- Weight accounting is spread across
set(zeroes the weight),weigh(evicts, possibly the caller's own key) andtrim(recomputes the group total from scratch). - Every refusal path decides a peer-visible status, and a lost group is reported to the application as traffic gone. An off-by-one there is silent data loss.
- Weight accounting is spread across
-
Expected hard, found easy:
- The codec tables (
defs/commands.ts,defs/tlvs.ts) and the TLV typing, includingtlvSpecskeying each definition to its own name. PduFramerandPduTransport.- The DLR parsing in
dlr.ts: every regex and every fallback has a one-line reason. SessionLifeitself, thanks to the diagram.- GSM packing (153 vs 134). The
segmentUnitscomment, plus the AGENTS section "GSM 7-bit is sent unpacked", made it obvious even to someone who knows nothing about SMPP.
- The codec tables (
-
Prose debt
- What I needed and what it cost:
- The README "SMPP terms" table (ESME/SMSC,
esm_class, UDH,sar_*). Cheap to find, essential without domain knowledge. - The README sections "Sends and the link", "Receiving in depth" and "Server in depth", to confirm the intent behind items 1 and 4. Each took a scroll-and-search.
- The AGENTS architecture map, for navigation.
- Several
// SMPP 3.4 x.y.zcomments point at a spec I have never read, and I had to take them on trust. Examples:respIdParams,refusalAnswer,messageClassOf. - The AGENTS decision index gives titles only. Twice (the drain ordering, and
data_codingownership in the codec) the title told me a rule existed without telling me the rule, and the reasoning lives indocs/decisions.md, which I was told not to open. - I opened no tests.
- The README "SMPP terms" table (ESME/SMSC,
- Prose that told me nothing the code did not:
- For reading
src/: the AGENTS test-conventions bullets (fixtures,resume(),t.afterordering) and the "Defects found in 0.4.0" table, which is history about another codebase. - The README Goals and Audience.
- A few restating doc comments:
PendingRequests.deliver("False means nobody was"),LinkTimers.clear's neighbours,defs/index.ts's grouping. - The duplicate
emit/captureRejectionSymbolguard comments insession.tsandserver.ts.
- For reading
- What I needed and what it cost:
-
Scores. Intrinsic difficulty is moderate to high (wire protocol, reconnect, backpressure, multipart), and it gets no bonus below.
- Navigation 8. Between 7 and 9: the AGENTS file map plus one concept per file (
dlr-merger.ts,link-waiters.ts,pdu-refusal.ts) got me from a symptom to the right file cold every time. The one detour was the second reconnect loop living inclient.ts. - Locality 6. Between 5 and 7:
SessionLifedoes centralise the state, but the answered-or-not state spansIncomingRequests,HandledMessagesandSms.ExpiringGroupsalso pushes enforcement of its own invariants onto three owners, and link identity is a sharedSocketreference compared across modules. - Shape 7. Predictable: fan-out is bounded (
Sessionwires about six collaborators through narrow option objects) and most names tell the truth. It is held there by a few that lie or hide effects:closing()mutates, thesettlecallback inkeepTryingdoes not settle, andIncomingRequests.clear()also empties the handled messages. - Self-sufficiency 7. Predictable: nearly every non-obvious line carries a one-line why, often with a spec reference, and the hard corners are marked. It stays below 9 because several rules (codec
data_codingownership, drain ordering) exist in the code as outcomes whose reasons are only indexed titles, and the domain vocabulary needs the README glossary open. - Overall 7. Capped at 7 by Locality 6. A cold mid-level reader knows within a day which corners to fear (items 1, 2 and 3 above). They are few and marked, but item 1 is harder than the problem requires.
- Navigation 8. Between 7 and 9: the AGENTS file map plus one concept per file (
SCORES nav=8 loc=6 shape=7 self=7 overall=7
Draft E, senior seat
Comprehension panel report: senior maintainability seat, @larvit/smpp (draft-e), whole project
I read README.md, AGENTS.md and every file under src/, including defs/. I opened no tests.
1. Hardest places, hardest first
-
src/reassembly.ts:187Reassembler.trim(), andsrc/expiring-groups.ts:70ExpiringGroups.weigh()weigh()can evict the group that is being weighed.trim()then has to work out whether that group survived. It also counts onlyparts.size - 1segments as lost when that group is the victim, because the new segment "stays with the peer".- To get this right I had to hold four things at once:
weigh's eviction order,takeOldest → delete → idle, the "answered" arithmetic, and the caller incollect(), which answersfull. - The inline comments made it clear in the end, after two passes.
-
src/expiring-groups.ts:19ExpiringGroups, as a contract- The class docstring says it "enforces neither max nor timeout itself", so every owner has to call
fullandtakeExpired()in the right order. HandledMessagesstretches this across two classes.IncomingRequests.onMessage(incoming-requests.ts:218) callshandled.refuses()before reassembly, andoffer()comes later, when the message is whole.- Nothing states that the cap holds only because
refuses()ran first. I had to reconstruct that myself, and it stayed implicit.
- The class docstring says it "enforces neither max nor timeout itself", so every owner has to call
-
src/session-life.ts:115SessionLife.enter()/:175attempt(), withsrc/session.ts:260lifeFor()- The ASCII transition table in the header is the best piece of prose in the repo.
- Two things cost me:
- The header says "a transition from any other state is ignored", but
enter()has no guard. The guards live in the public wrappers (bound(),closing(),linkLost(),end()), so I had to check each one. - The effects are closures defined in
Session. To followdown → connected → boundI had to jump betweensession-life.ts,session.tslifeFor/linkDown,OutgoingRequests.linkUp/linkLostandPduTransport.attach.
- The header says "a transition from any other state is ignored", but
attempt()'s re-check after the await (this.links !== link || !this.is('connected')) is commented and fine.
-
src/client.ts:240retryUntilBound()/:278keepTrying()- This is a second backoff loop, separate from
SessionLife's. The charter and README sayfromStartgoes "through that same loop", but it doesn't: it's a separate loop that uses the sameBackoffclass. - The callback type is named
Settle, but on an error it doesn't settle anything; it only recordslastErr. You learn that from a comment inside the lambda inkeepTrying. - Add the
waiting()closure and an abort listener, and this is four nested pieces of control flow for one feature. It resolved in the end, but the name misled me along the way.
- This is a second backoff loop, separate from
-
src/pdu.ts:84resolveShortMessage()/:113resolveBody()- The
CodingSourcerule decides which ofshort_messageandmessage_payloadmay rewritedata_coding. It depends on whether the command's table has ashort_messageat all, on Buffer vs string, and on zero length. - The comment on the
CodingSourcetype states the rule, but I had to trace three return shapes to confirm it. - Next to it,
readParams()(:240) passessm_lengthas the length argument to every param read. That works only becausesm_lengthcomes earlier in wire order.commands.tsdocuments wire order, but not that this depends on it.
- The
-
src/sms.ts:126sendResp(), withsrc/session.ts:304answer()sendRespchecks the socket the message arrived on,link.destroyed.answer()then writes totransport.sock, the current socket.- This is correct only because a socket is replaced only after it has been destroyed (
PduTransport.attach). IncomingRequests.handle()(:104) relies on the same equivalence. It is not stated anywhere I read.
-
src/handled-messages.ts:120HandledMessages.run()- Covers expiry, a handler failure, answering after the handler settles, and the identity check
running.get(key) !== sms, which catches aclear()or sweep that ran in the meantime. - The class docstring covers it. It is compact but dense.
- Covers expiry, a handler failure, answering after the handler settles, and the identity check
-
src/defs/encodings.ts:163messageClassEncoding()/encodingByDataCoding()- Bit-twiddling over GSM 03.38 coding groups that I had no background for.
- The comments are adequate. The problem is inherently hard; it is not badly written. Stacking a
//comment on top of a/** */comment made it unclear which one belonged to which function.
2. The unit I would least want to modify
Reassembler.trim() together with ExpiringGroups.weigh().
- The eviction, the "is the current group gone" question, the lost-segment count and the ESME-facing status (
full→ throttled) are all spread over two files, and each file assumes the other's behaviour. - A wrong change here silently loses traffic the peer will never resend, which is the README's worst outcome.
- Nothing in the code would stop me. Only a test would catch the mistake.
3. Expected hard, found easy
- Framing (
pdu-framer.ts): short, and the quadratic-join rationale is right there. - The send path:
OutgoingRequests.request/sendOnce/attempt. Thewrittenflag makes "resend or not" a single boolean. PendingRequestsandSendWindow- The TLV table's self-keyed generic
- Receipt parsing (
dlr.ts) splitMessage: the budget comment together with the charter's "GSM 7-bit is sent unpacked" section settled the 153/134 question at once.
4. Prose debt
Needed:
- The
session-life.tsstate table: essential, and cheap to find. AGENTS.md's file map: accurate, and the fastest route from a symptom to a file.- The charter's "GSM 7-bit is sent unpacked" section.
- README "Server in depth", to understand why segments are answered on arrival.
Missing:
- The invariants in items 2 and 6. They are written down nowhere I was allowed to read.
- The decisions index points at
docs/decisions.md, which I couldn't open. Twice (thefromStart"same loop" wording, and the abort-dance duplication) the one-line index entry made a claim the code did not obviously bear out, and there was nothing local to check it against.
Prose that told me nothing new:
- The
/** Injected so expiry can be exercised without a wall clock. */comment, repeated on four option types. // Called unbound, so the application's hook never sees this class as its this(incoming-requests.ts).- Most of the charter's test-convention paragraph (fixtures,
recordingDeps), which is irrelevant for readingsrc/. - Much of the defect table, as far as reading
src/goes.
5. Scores
- Navigation 7: Predictable. The
AGENTS.mdfile map is accurate line by line, and file names match their content (link-waiters,pdu-refusal,sms-id), so I landed first try on every symptom I tried. It doesn't reach 9 becausedefaultsare applied in three different layers (Session,IncomingRequests,Reassembler), so finding "where does this default apply" takes a search. - Locality 6: Between honest middle and predictable. It is held down by three unwritten cross-class invariants: owners enforce
ExpiringGroups' limits,refuses()must run beforeoffer(), and a destroyed arrival socket stands in for the current socket.SessionLife's effects are also closures back intoSession. - Shape 7: Predictable. Every file is small, fan-out per level is bounded, and names mostly tell the truth. It is kept from 8 by a few names that mislead:
closing()is a transition, not a predicate.Settledoesn't settle on an error.bound()exists on two layers with different contracts.onRequestnames both the server's wrapper and the application's hook.
- Self-sufficiency 7: Predictable. The why-comments sit on the lines that need them, and spec section numbers are cited. It is held back by a charter index that points to a decisions file whose claims I could not check locally.
- Overall 6: The capped maximum is 7 (lowest dimension plus one). I gave 6 because the costs are concentrated in exactly the bounded-store and link-identity code where a mistake loses traffic.
The problem itself is moderately hard: flow control across two peers, the SMPP body and encoding rules, and reconnect races. That earns no bonus.
SCORES nav=7 loc=6 shape=7 self=7 overall=6
Draft E, architect seat
Comprehension panel, seat "Architect, inherited": @larvit/smpp (draft-e)
I read README.md, AGENTS.md and every non-test file in src/. I opened no test, ran nothing, edited nothing and ignored the pre-loaded AGENTS.md.
1. Map from README.md and ls -R src test only (verbatim)
Flat
src/of 36 files plusdefs/(7). I expect seven areas that the layout does not show: A. Spec tables,defs/: commands, constants, encodings, errors, TLVs, wire types, plus an index grouping them. B. Codec:pdu.ts(pduToObj/objToPdu),pdu-framer.ts(stream to PDUs),pdu-refusal.ts(PduRefusedError),retained-pdu.ts(?),result.ts,error-from.ts(?). C. Message content:message.ts(encode/split/bitCount, and probably smppTime),message-body.ts,concat.ts,udh.ts,reassembly.ts,expiring-groups.ts(a TTL map, probably under reassembly). D. Receipts:dlr.ts(parse and build receipts),dlr-merger.ts(messageDlr),sms-id.ts(hex/decimal ids). E. Session core:session.ts,session-life.ts(?),session-options.ts(types),defaults.ts,link-timers.ts(enquire_link/idle),backoff.ts(reconnect loop?),pdu-transport.ts,link-waiters.ts(?),idle-waiters.ts(?). F. Request flow:outgoing-requests.ts,pending-requests.ts(seq/correlation),send-window.ts,unanswered-error.ts,send-sms.ts,incoming-requests.ts,sms.ts(the onSms handle),handled-messages.ts(messages whose onSms runs, for the bound and the drain). G. Entry points:client.ts,server.ts,index.ts,log.ts,uuid.ts. Names that do not give their purpose: session-life, link-waiters vs idle-waiters, retained-pdu, error-from, defaults vs session-options, and backoff (a loop or only a delay?). Expected from the README but missing: the goal-9 store (the README says it has not shipped). At the repo root, MIGRATION-NOTES.md and DESIGN.md sit beside the six documents the charter names, with no stated purpose.
Corrections, cheapest first
| Map guess | Reality | Cost |
|---|---|---|
| backoff.ts is the reconnect loop | Delay arithmetic only. The loop is the down state of SessionLife. |
Low. The AGENTS table says so. |
| link-waiters / idle-waiters | Requests waiting for a bound link / a count falling to zero | Low |
| dlr.ts parses and builds receipts | Building lives in sms.ts (receiptText, sendDlr, collectReceipt) |
Medium. I went to dlr.ts first, then grepped for stat:. |
| session-options.ts is option types | Also holds SessionEvents, bind-direction logic (bindCarries, standsInFor, bindCommands) and all option validation (checkSessionOptions) |
Medium. The name hides three concerns. |
| Whole-message decoding lives in message.ts | decodeSegments is in reassembly.ts:63 and is called from sms.ts |
Low-medium |
| One reconnect loop | Two. SessionLife.attempt/schedule, plus a second in client.ts:240 retryUntilBound/keepTrying for fromStart. Both log 'reconnect - retrying'. |
Medium. The same log line from two places is a 3am trap. |
| retained-pdu, error-from | Heap-detach and weight of a held PDU; turning a thrown value into an Error | Low. The AGENTS table covers both. |
2. Fan-out by level
- Repo root: about 8 prose documents (README, AGENTS, CHANGELOG, MIGRATION, MIGRATION-NOTES, DESIGN, todo, CLAUDE) plus 5 directories. MIGRATION-NOTES and DESIGN are not in the charter's "each file answers one question" list.
src/, the worst level: 37 entries. They fall into 7 real areas, but only the AGENTS.md table shows that. Without the table this level is past any bound you can hold at once.src/defs/: 7. Good.- Within the session:
Sessionholds 8 collaborators.OutgoingRequestsholds 3 (PendingRequests, LinkWaiters, SendWindow).IncomingRequestsholds 2 (HandledMessages, Reassembler) plus 6 injected fields.HandledMessagesholds 2 (ExpiringGroups, IdleWaiters).- Depth is at most 4 and fan-out at most 8 per level. Bounded.
- Files: most are under 250 lines.
defs/types.ts(684 lines) is long but uniform: one wire type after another.
3. Names
Misleading:
SessionLife.closing()(session-life.ts:97) reads like a predicate but performs the transition and returns whether it did.carries()andattached()next to it really are predicates.session-options.tsholds events, bind direction and validation as well as options.- The comment on
SessionOptions.shutdownTimeout(session-options.ts:117) says "How long a drain waits for the requests already on the wire". It also bounds the wait on running handlers (session.ts:324). That comment is false. - In
client.ts:234,288,Settleis called once per failed attempt as well as on success, so it does not settle anything. maxOctets(a public option) bounds reassembly only. The handled-message octet cap is a separate constant,maxHandledOctets.
One name over several concepts:
idlehas four meanings:- A count reaching zero:
IdleWaiters,SendWindow.idle,HandledMessages.idle. - Stopping the sweep timer:
ExpiringGroups.idle(). - The idle-timeout timer:
LinkTimers.idle. - The
idleTimeoutoption.
- A count reaching zero:
settlehas four meanings: wake all waiters (IdleWaiters.settle), wake only if the count is zero (HandledMessages.settle), resolve one request (PendingRequests.settle), and the localsettleclosures in LinkWaiters, SendWindow and client.linkmeans the socket (SmsInput.link,const link = this.session.sock), the connection's lifetime (LinkState,LinkTimers,LinkWaiters) and which end of the connection this is (LinkEnd).
Two names for one concept:
- The lost link is spelled
linkLost(the SessionLife transition andOutgoingRequests.linkLost) andlinkDown(the LifeEffects hook andSession.linkDown). - The end of the session is spelled
end(), the stateended, and the effectover. - A message whose handler is running is spelled "handled" (the class and its logs),
running(its field) and "being handled" (README).
4. What I would restructure, ranked
- Group
src/into about 6 directories:codec/(pdu, framer, refusal, retained-pdu, defs),message/(message, message-body, concat, udh, reassembly),receipts/(dlr, dlr-merger, sms-id, and receipt building moved out of sms.ts),session/(session, session-life, link-timers, pdu-transport, backoff),requests/(outgoing/pending/send-window/link-waiters/idle-waiters, incoming/handled-messages/sms), and top level (client, server, index). Today the AGENTS table does the job the directory tree should do. - One retry loop. Have
fromStartreuse theSessionLifeloop, or give client.ts's loop a distinct name and log line. - Split
session-options.tsintobind-direction.tsandoption-checks.ts, and moveSessionEventsintosession.ts. - Retire the overloaded verbs (
idle,settle,linkLost/linkDown) and renameclosing()to something likebeginDrain(). - Deduplicate the collectors.
collectReceipt(sms.ts:182) iscollectSent(send-sms.ts:274) without ids.
What the structure gets right:
SessionLifeis one state value with the transition diagram in its own header (session-life.ts:7-24). Effects reach the rest of the session only throughLifeEffects.OutgoingRequestsreads state through a function and never copies it.- Every collaborator takes a narrow options object.
- Result types are used throughout, so control flow has no second error channel.
- The comments are dense and mostly say why, not what.
- The AGENTS table matches the code file for file.
5. The 3am question
Symptom: during a graceful shutdown the session hangs until the shutdown timeout, even though the application already called sms.sendResp() on every message.
Cold path, about 3 to 5 minutes:
Session.close(session.ts:235) callsdrain(session.ts:317).- That calls
this.incoming.drain(timeout)(incoming-requests.ts:163). - That calls
HandledMessages.idle(handled-messages.ts:106). - The unit is
HandledMessages.run(handled-messages.ts:120). A message leavesrunning, which is what releases the drain, only afteronSms's promise settles.sendResp()records the answer and releases nothing.
Likely cause: the handler is still awaiting something, typically sms.sendDlr(). That goes through handlers.send, which is outgoing.request: it bypasses the closing-state refusal and waits up to responseTimeout (30 s), which is longer than shutdownTimeout (5 s).
What gets you there: the README (lines 107-109) and the OnSms doc (session-options.ts:87-92) both say the handler's promise is the hold. What slows you down: the Sms.sendResp doc (sms.ts:47-51) says nothing about the hold, and the shutdownTimeout comment wrongly claims it only bounds requests.
Where it rots first: the wiring in Session's constructor and transportFor/lifeFor/incomingFor (session.ts:105-294).
- Eight collaborators are cross-wired through closures that capture
this.lifeandthis.transportbefore those are assigned. That is safe today only because of construction order. IncomingRequestsreaches back intoSessionforsock,close(),emit,bindAllows,boundAsandlinkEnd, so the dependency runs in both directions.- Every lifecycle feature touches three files: session-life (transition), session.ts (effect) and the collaborator.
Where the next two features land:
- The goal-9 store lands under
ExpiringGroups, which has three owners:DlrMerger,ReassemblerandHandledMessages. Each wraps it differently (weigh-evicts, a "spent" set, sweep callbacks), so a persistent seam would have to be cut three times orExpiringGroupswould have to become the store interface. - A per-PDU rate-limit hook lands in
OutgoingRequests.sendOnce(outgoing-requests.ts:116), between the window acquire andattempt, and must respect the rule that a written request is never retried. The code states that rule, so the change is contained. An alphabet hook would be far worse:EncodingNameis a closed union that ripples through defs/encodings, message.ts and send-sms.ts.
6. Hardest places, ranked
session.ts:105-294,Sessionconstructor plustransportFor/lifeFor/incomingFor: closure wiring, and initialisation order matters.session-life.ts:175,SessionLife.attempt: re-enters after two awaits and guards with thelinkscounter plusis('connected'). It is correct, but you have to hold the whole diagram to read it.session.ts:218,Session.unbind: thewasOpen/closedOnUnbindarithmetic and the order of error precedence.outgoing-requests.ts:97-183,request/sendOnce/attempt: afor(;;)retry keyed onwrittenand a freshstate()read.handled-messages.ts:62,120,refuses()(a query that mutates the hysteresis flag and sweeps) andrun()(answer after settle, then conditional delete).pdu.ts:84-136,resolveShortMessage/resolveBody: which field's encoding setsdata_coding.reassembly.ts:187trimwithexpiring-groups.ts:70weigh: eviction can take the current key, with off-by-one accounting of lost parts.client.ts:240-309,retryUntilBound/keepTrying: the second loop and its misnamed settle callback.
The unit I would least want to modify is SessionLife.enter (session-life.ts:115) together with its effect bindings in Session.lifeFor (session.ts:260). One transition's meaning is split across two files and five callbacks.
Intrinsic difficulty is high, and none of the scores below credit it: an async protocol with reconnect, drain, correlation, reassembly and a byte-exact codec against peers that do not follow the spec.
7. Scores
- Navigation 7: at the "Predictable" anchor. The AGENTS file table plus accurate file names got me from the 3am symptom to
HandledMessages.runin minutes. It sits no higher because the 37-file flatsrc/depends on that table, and two parallel retry loops share one log line. - Locality 6: between "Honest middle" and "Predictable".
LifeEffectsand thestate()accessor are real seams. Holding it down:IncomingRequestsreaches back intoSession, the constructor wiring depends on order, and one lifecycle change spans session-life, session and a collaborator. - Shape 6: between 5 and 7. Nesting below the top level is bounded (8 at most per level). Holding it down: the 37-entry top level, a misnamed
session-options.ts, a mutatingclosing(), andidle/settle/linkeach covering several concepts. - Self-sufficiency 7: at "Predictable". The state diagram, invariant comments and "why" comments sit at the code. Holding it back from higher: the false
shutdownTimeoutcomment (session-options.ts:117), andSms.sendRespnot saying it does not release the drain. - Overall 6: capped at 7 by the lowest dimension plus one. The hard parts are marked but spread across the session wiring, and the top level needs a document to map it.
SCORES nav=7 loc=6 shape=6 self=7 overall=6
Draft F, junior seat
Junior A: comprehension report for draft-f
I read README.md, AGENTS.md and every file under src/, all in draft-f. I opened no test.
1. Hardest places, hardest first
-
src/expiring-groups.ts:38-54, 111: theExpiringGroupsgettersfull,sizeandweight, andget(), which all callexpire().- Reading a property has side effects. It can fire
onDrop. - In
Reassembler,onDropbecomesonLost, thenport.report, then asessionErroremit. InRunningHandlersit logs a warn and settles the drain's waiters. - So
this.running.sizeinIncomingRequests.refusedAtBound(incoming-requests.ts:224) can emit events on the application's emitter. I only found this by followingonDropthrough three classes. - No comment at the call sites says so. It stayed a trap even after I understood it.
- Reading a property has side effects. It can fire
-
src/reassembly.ts:113(Reassembler.collect) withweighed()at:180,dropped()at:194and theweighingfield at:91.weighingis a side channel. It is set around aweigh()call so that the drop callback, which fires synchronously inside it, can subtract the newest segment from the reported loss.weighed()then readsget(key)again to find out whether its own group was evicted.- To follow it I had to hold several things at once:
- part and total validation;
- a group that is new or already there;
- eviction by count inside
set; - eviction by weight inside
weigh; - the "newest segment stays with the peer" rule.
- The field's comment explains why but not how. It was resolved only after a second read.
-
src/incoming-requests.ts:260(onMessage) and:292(handOver), together withsms.ts:126(sendResp) and:196(sendDlr).- A message's answer is spread over four places:
- answered on arrival for segments;
- the handler's return;
- an early
sendResp(); - the refusal on a throw, which is itself split by
answeredOnArrival.
- The shared state is the mutable
answerobject captured in the closures ofcreateSms(sms.ts:80). throttledStatusversusrefusedSegmentStatusneeds thecarriedAs/standsInForindirection fordata_sm.- The README's "Receiving in depth" section resolved it. The code alone did not.
- A message's answer is spread over four places:
-
src/pdu.ts:84(resolveShortMessage) and:113(resolveBody).CodingSourcedecides which ofshort_messageandmessage_payloadmay rewritedata_coding.- There are five return paths, depending on Buffer or string, empty or not, and a string
message_payload. data_codingis patched onto the params from two places.- The type comment at
:74helps, but I had to trace each branch by hand. It remained partly opaque, for example why an empty encoded string falls tomessage_payload.
-
src/defs/encodings.ts:152-191:messageClassOf,messageClassEncoding,encodingByDataCoding.- Bit masks (
& 0x80,>> 2 & 0x03,0xF0) that I had no background for. - Two comments are stacked oddly at
:160-162: a//block and then a/** */, both describing the function below. - It stayed opaque without the GSM 03.38 spec. The intrinsic difficulty is high.
- Bit masks (
-
src/defs/tlvs.ts:110(WriteValue) and:284(keyedTlvs, with theisTlvsguard).- Conditional types nested three deep, plus a runtime re-validation of what the code just built, because casts are banned.
- Hard rule 4 in AGENTS.md explains why the guard exists. The type gymnastics remained costly.
-
src/reconnect-loop.ts:82(schedule),:127(attempt) and:64(adopt).upAtresets the backoff only after a link lastedmaxDelay.links > 0decidesunref.- A
stoppedcheck comes after anawaitinattempt. - There are three flags (
timer,attempting,stopped) guarding re-entry. - The comments explain each rule, so it was resolved, but it is order-sensitive.
-
src/send-window.ts:42(release). Handing a slot to a waiter without decrementinginFlightis correct but not commented. I had to reason out that the slot transfers.
2. The unit I would least want to modify
Reassembler.collect / weighed / dropped. A change to eviction order inside ExpiringGroups.weigh, or to when get() expires, silently changes the loss counts reported to the application, which are sessionError events. Nothing at the call site tells me that the coupling exists.
3. Expected hard, found easy
PduFramer: short, one clear purpose.PendingRequestsandOutgoingRequests: the split betweenLinkLostError("never written, retry") andUnansweredError("may have been taken") is named well.SmppClient.send's retry loop (client.ts:143) read at once.- The layering of Session, IncomingRequests and SessionPort: the port type (
incoming-requests.ts:50) says exactly what the collaborator may touch. server.tshandleRequest: bind-before-anything is compact.defs/types.ts: long but repetitive and uniform.
4. Prose debt
What I needed, and what it cost to find:
- README "Glossary": needed for ESME/SMSC,
esm_class,data_coding, UDH andsar_*. It sits about 600 lines down the README and nothing insrc/points at it. - README "Receiving in depth" and "Server in depth": needed to understand the answer-on-arrival model before
onMessagemade sense. - AGENTS "GSM 7-bit is sent unpacked": needed to believe the 153/134 budget in
message.ts(thesegmentUnitsconstant near the top). - The
ASCIIencoding name: it means GSM 03.38. Only the comment inencodings.tsnear line 45 and a README table say so. The name lies. - Bit layout of
esm_classanddata_coding: not documented anywhere I was allowed to read. I would need the spec.
Prose that told me nothing the code did not:
- AGENTS' architecture map repeats most file-level doc comments almost word for word. It was still useful as an index.
- Several Conventions paragraphs about tests (
dummy-smsc,recordingDeps) were irrelevant to readingsrc/. - AGENTS' decision index names
ReconnectOptions, which does not exist insrc/. The type isReconnectTuning. That line is stale. - Comments that add nothing:
'Whether the socket has closed.'onclosed(session.ts:148);'Sends a request and resolves with the peer's response.'(session.ts:172);'Connects to an SMSC and binds.'(client.ts:414).
5. Scores
| Dimension | Score | Anchor and cause |
|---|---|---|
| Navigation | 7 | Predictable. AGENTS' one-line-per-file map and honest file names (pdu-framer, dlr-merger, send-window) took me from a symptom to a file first time. ASCII meaning GSM and the three meanings of "refuse" and "answer" (Session.refuse, IncomingRequests.refuse, sendReturn / answer / sendResp) keep it off 8. |
| Locality | 5 | Honest middle. The getters in ExpiringGroups fire callbacks that emit on the session. The weighing side-channel field depends on synchronous re-entry. The mutable answer closure is shared by sendResp and sendDlr. Changing one piece means holding its callback chain. |
| Shape | 6 | Between honest middle and predictable. Fan-out is bounded (Session builds four collaborators; IncomingRequests builds two). Some names mislead: ASCII; stopping, which the peer's unbind also sets; over versus closed; SessionListener's emit guard copied three times. |
| Self-sufficiency | 6 | Between honest middle and predictable. Inline spec citations ("SMPP 3.4 5.3.2.26", "4.6.2") and why-comments mostly carry it. The answer-on-arrival model and the data_coding bit groups still need the README or the spec open beside the code. |
| Overall | 6 | Capped by Locality (5 + 1). |
Intrinsic difficulty: the problem is hard, and gets no bonus in these scores. It involves concurrency, the protocol's split between UDH and sar_*, receipts that look like messages, and the GSM alphabets.
SCORES nav=7 loc=5 shape=6 self=6 overall=6
Draft F, mid seat
- Hardest places, ranked hardest first
-
src/reassembly.ts:330Reassembler.weighed()/dropped()(:344), together withsrc/expiring-groups.ts:38-54(thefull,sizeandweightgetters).weighed()setsthis.weighingso thatdropped(), reached re-entrantly throughExpiringGroups.weigh()→dropOldest()→onDrop, knows to subtract the newest segment from the loss count. That is a side channel through a field. On top of it, every getter onExpiringGroupsrunsexpire(), which firesonDrop. So readingsizein a log line (reassembly.ts:358,this.groups.weightinsidelost()) can drop other groups and report them mid-report. The comments at :240 and :274 got me to what it intends. Whether the re-entrant drops are harmless stayed unresolved. -
src/session.ts:211-332Session.unbind()/drain()/finish()/end(). Three flags carry a lifecycle:over,stoppingand theendedpromise.closedis a getter overover, andport().end(:262) setsstoppingfrom outside the drain.unbind()'sdroppedOnUnbinddepends onUnansweredErrorandthis.overagreeing after an await. The field comments (:64, :66) and the class doc resolved which flag means what, but only after I built a table by hand. -
src/client.ts:515SmppClient.send(). It loops onLinkLostError. Whether it terminates depends onReconnectLoop.bound()(which hands back the current session until the socket'scloseevent),Session.send()(which checksover, set only onclose) andPduTransport.write()(which fails onsock.destroyed). A socket that is destroyed but has not yet emittedcloselooks to me like it gives a loop that resolves only through microtasks and may never yield. I could not rule that out without a test. This is the plainest action-at-a-distance in the codebase, and it stayed opaque. -
src/incoming-requests.ts:260-351onMessage()→handOver()→refuse(), withsrc/sms.ts:796-860createSms()/sendResp(). I had to hold several things at once:- whether the message was answered on arrival;
- whether the handler threw;
- whether it returned
{ smsId }or{ status }; - the
Answerobject mutated inside the closure; carriedAschoosing the retry status.
alreadyAnsweredhas four error texts for these combinations. The README section "Receiving in depth" resolved it; the code alone did not. -
src/defs/encodings.ts:152-191messageClassOf(),messageClassEncoding(),encodingByDataCoding(). This is bit arithmetic on a GSM 03.38 layout I have never seen. There is a//comment and then a/** */comment stacked on one function (:160-162), and the//one reads as though it belongs to the function above. The comments give the bit positions. Why 0x03 means one thing below 0x80 and another in a class group stayed half opaque. -
src/dlr.ts:157-238messageType()/receiptStatus()/dlrFromPdu().'unmarked'versus'receipt', whether the TLV or the body wins, andstatusMsgpossibly beingundefinedbefore it falls back to'UNKNOWN'. The README's Delivery receipts section resolved it. Without the README I would have guessed wrong about'other'. -
src/pdu.ts:323-375resolveShortMessage()/resolveBody(). TheCodingSourcenaming feels inverted: an emptyshort_messageyieldssource: 'message_payload', meaning "message_payloadmay setdata_coding". The comment at :313 explains it, but I had to read it twice. -
src/reconnect-loop.ts:83schedule(). It resets the backoff only ifupAtshows the link outlastedmaxDelay, clearsupAt, doubles the delay after capturing it, and callsunref()only once a link has existed. Each step has a comment, which resolved it. It is dense rather than opaque.
-
The unit I would least want to modify:
ExpiringGroups(src/expiring-groups.ts). Three owners (Reassembler,DlrMergerwith itsspentstore, andRunningHandlers) depend on when drops happen and in what order. Reads mutate state and fire callbacks.set()can evict.weigh()can evict the entry being weighed. Any change moves loss accounting, drain wake-ups and receipt-merge refusal all at once. Note also thatRunningHandlerslogs "giving up on a handler that never returned" for an eviction, not only an expiry. -
Expected hard, found easy
- The wire codec:
defs/types.ts,pdu.tsparse/build, TLV read/write. It is long but regular: every reader range-checks, and every error names the parameter. PduFramer.PendingRequestsandSendWindow.- The server's bind handling (
handleRequest). - The option validation in
session-options.ts.
Result-typed code with no throws made control flow easy to follow.
- The wire codec:
-
Prose debt
-
Needed:
- the README Glossary for ESME, SMSC,
esm_class,data_coding, UDH andsar_*(cheap to find, essential for me); - the README "Receiving in depth" and "Server in depth" sections, for the answer rules in
sms.tsandincoming-requests.ts(about 10 minutes to locate); - the AGENTS "GSM 7-bit is sent unpacked" section, for
segmentUnits153 versus 134.
The AGENTS decision index names choices, such as "A message id base is merged at most once", whose reasoning lives in
docs/decisions.md, which I was not allowed to open. For a few (thespentstore andLinkLostErrorretry), the title alone left me unsure whether the behaviour I saw was the intent. The charter also saysIncomingRequestsandSmsget "named functions, never the session itself". That is false as written:SessionPort.sessionandSms.sessionboth hand over the fullSession(incoming-requests.ts:65,:293). - the README Glossary for ESME, SMSC,
-
Told me nothing:
- most AGENTS architecture one-liners, which restate the file names;
- the seven
declarelistener lines, repeated in all three emitters; /** Whether the socket has closed. */onclosed;defaults.ts's "README's option tables restate the public ones";- many decision-index bullets that simply restate what the code shows, such as "
reconnecttakes{ minDelay, maxDelay }…".
Intrinsic difficulty: moderate to high. Two alphabets' bit layouts, two concatenation spellings, and receipts sharing a command with messages are the problem's own difficulty, not the code's. They get no bonus.
-
-
Scores
- Navigation: 7. At the "predictable" anchor:
src/is flat, file names follow concepts (dlr-merger,pdu-framer,send-window), and the AGENTS map matched the tree exactly. It stops short of 8 because of placements likeleftOfinidle-waiters.tsandrefusedSegmentStatusexported fromincoming-requests.ts. - Locality: 5. At the "honest middle": getters with side effects in
ExpiringGroups, theweighingside channel inReassembler, andSmppClient.send()being correct only across the timing of three modules are all state changed out of sight. - Shape: 6. Between 5 and 7. Files are small and fan-out is bounded, but the vocabulary is overloaded:
- answering:
answer,sendReturn,sendResp; - refusing:
refusein two classes with different meanings, plusrefusedAtBound; - ending:
over,closed,stopping,end,finish,ended; - and
SessionPortclaims a narrow seam while carrying the wholeSession.
- answering:
- Self-sufficiency: 6. Between 5 and 7. Comments cite SMPP sections and state the why in place (for example
respIdParamsandrefusalStatus). But the answer rules and receipt classification needed README sections open beside them, and the decision index points at reasoning that is not in the code. - Overall: 6. Capped by locality (5 + 1). The code reads cleanly line by line; what costs is the few places where state moves out of sight.
- Navigation: 7. At the "predictable" anchor:
SCORES nav=7 loc=5 shape=6 self=6 overall=6
Draft F, senior seat
-
Hardest places, ranked
-
src/incoming-requests.ts:260-351,IncomingRequests.onMessage/handOver/refuse, plusrefusedAtBoundat :224. Before I could predict the answer to one inbound PDU, I had to hold eight branches at once:- the running-handler bound;
- concatenated or whole;
- kept or refused, and full or unplaceable, with the refusal status differing between sar and UDH;
- whole or partial;
- link already closed;
- no
onSms; - the handler threw or returned;
- answered on arrival or not.
"Who answers the peer, and when" is spread over
onMessage,handOver,refuse,createSms(answeredAs)andsms.sendResp/alreadyAnsweredin another file (sms.ts:106-144).refusedAtBoundreturns a boolean but also writes the answer and flips therefusingflag. The comment at :256 and README "Receiving in depth" / "Server in depth" resolved it, but only after two passes. -
src/reassembly.ts:91,113-137,180-198,Reassembler.collect/weighed/dropped. Theweighingfield is a side channel. It is set aroundgroups.weigh()so that the synchronousonDropcallback, which re-entersdropped(), can subtract the newest segment from the reported loss.collectalso inserts the segment intogroup.partsbefore it knows whether the group survives the weighing. The comments at :90 and :124 made it resolvable, but only by tracing the re-entrancy by hand. -
src/expiring-groups.ts:250-266,295-306,323-334, theExpiringGroupsgettersfull/size/weightandweigh. Reading a property runsexpire(), which firesonDropcallbacks. ThroughRunningHandlers(running-handlers.ts:259) that means a plain read ofthis.running.sizeinside a log call inrefusedAtBound(incoming-requests.ts:229) can log a "giving up on a handler" warning and wake a drain. The class comment says "the expired go on every access". Nothing at the call sites marks it. This stayed partly opaque: I am not certain every caller tolerates it. -
src/session.ts:211-221,291-332,Session.unbind/drain/finish/end.- There are three lifecycle flags:
over,stoppingandbind. stoppingis also set from outside, throughSessionPort.end(:262).drainreads the same state asthis.overat :294 and asthis.closedat :303.unbinddeliberately bypassessend()'s stopping check by callingoutgoing.requestdirectly, uncommented.droppedOnUnbindneeded its docblock plus the charter's decision index ("a close arriving after our own unbind is clean") before I trusted it.
- There are three lifecycle flags:
-
src/client.ts:143-156,202-217,415-438withsrc/reconnect-loop.ts:64-153,SmppClient.sendretry loop /takeFirst/keepTrying/ReconnectLoop. TheLinkLostErrorcontract spans four files.send-window.closesets it for queued requests andOutgoingRequests.attemptsets it on a write failure (outgoing-requests.ts:269,287).SmppClient.sendconsumes it, withReconnectLoop.bound/releasein between. The first session is opened outside the loop and thenadopted.links === 1means "first", andlinks > 0decidesunref(). Resolved by theLinkLostErrorclass doc and the README's "Sends and the link". -
src/pdu.ts:74-136,resolveShortMessage/resolveBody. The rule for which ofshort_messageandmessage_payloadmay overwritedata_codingusesCodingSource. An empty Buffer counts asmessage_payload, and only a non-empty encodedshort_messagerewrites the coding. I read it three times. TheCodingSourcedoc resolved it. -
src/defs/encodings.ts:476-515,messageClassOf/messageClassEncoding/encodingByDataCoding. This is bit arithmetic over GSM 03.38 coding groups that I had no background in. A//comment and a/** */for different concerns are stacked on one function (:484-486). The comments carry the rule, so it was domain cost rather than code cost. -
src/defs/tlvs.ts:103-126,284-306, theWriteValue/Repeatedconditional types andkeyedTlvs→isTlvs. The type layer takes a slow read. The runtime re-validation that ends in "a defect in this library" exists only to avoid a cast. I understood why only through hard rule 4 in the charter.
-
-
The unit I would least want to modify:
IncomingRequests.onMessage/handOvertogether withsms.tssendResp/alreadyAnswered. The invariant "every PDU gets exactly one answer, and no answer names an id or refusal after the segments were answered" is not stated in one place. It is enforced jointly by theansweredAsargument, the mutableanswerobject captured increateSmsclosures, theclosed()check placed aftercreateSms, andrefuse'sansweredOnArrivalbranch. A change to any one of these can double-answer or silently drop, and only a test would tell me. -
Expected to be hard, found easy:
- The codec:
pdu.tsread/write,defs/types.tswire types,pdu-framer.ts. It is long but uniform and bounds-checked the same way everywhere. PendingRequests,SendWindowandIdleWaiters: small, one job each.dlr.tsreceipt parsing, with the operator quirks commented inline.send-sms.ts: a linear checklist, then fan-out.- The server bind composition in
server.ts:508-533.
- The codec:
-
Prose debt
- Needed:
- README "Receiving in depth" and "Server in depth", for answered-on-arrival semantics and the retry statuses. Found by the table of contents, so the cost was low.
- The charter's architecture map. It was the most valuable document: every file is listed with a truthful one-liner.
- The decision index titles. They told me that behaviours like the unbind-close and five-minute handler were deliberate. With
docs/decisions.mdoff-limits, a title was sometimes all I had. - I opened no tests.
- Told me nothing new:
- The AGENTS.md "Defects found in 0.4.0" table (history, irrelevant to reading
src/). - The long test-fixture convention paragraphs.
- README "Methods", which lists names already typed.
- Comments that restate code:
session.ts:148"Whether the socket has closed."session.ts:172"Sends a request and resolves with the peer's response."server.ts:631"Starts listening… Resolves once the socket is bound."tlvs.ts:14"Ordered by tag id", which repeats the charter.
- The seven
declarelistener lines, copied across three emitters, are boilerplate rather than prose, but they are reading cost all the same.
- The AGENTS.md "Defects found in 0.4.0" table (history, irrelevant to reading
- Needed:
-
Scores
- Navigation 8: between "predictable" and "near duress-proof". AGENTS.md's file map matches
src/one-to-one, and names likepdu-refusal.ts,send-window.tsanddlr-merger.tslead from a symptom to the file first try. The detour is the answer path, split acrossincoming-requests.tsandsms.ts. - Locality 6: between "honest middle" and "predictable". The seams are named and narrow (
SessionPort,SmsDeps,SendSmsDeps,LinkLostError). Three things still break locality:ExpiringGroupsgetters fire callbacks on read.Reassembler.weighingis a re-entrancy side channel.Session's flag trio is mutated throughport.end.
- Shape 7: "predictable". No file is past about 440 lines and fan-out per level is small. A few names mislead:
ExpiringGroupsis used for running handlers and spent ids, which are not groups.closedandoverare two names for one state.refusedAtBoundanswers the peer as a side effect.ReconnectLoop.attempt's comment calls the library's own connect and bind "the application's".
- Self-sufficiency 7: "predictable". Comments carry the SMPP section and the why at the non-obvious points (
sms-id.ts:623,message.ts:13,dlr.ts:247-253,pdu-refusal.ts:446). The answered-on-arrival contract is the one thing that needed the README open beside the code. - Overall 7: capped at 7 by locality. A cold senior is productive within a week and knows which corners to fear. Intrinsic difficulty is moderately high (protocol quirks plus a concurrent drain and reconnect), and that earns no bonus.
- Navigation 8: between "predictable" and "near duress-proof". AGENTS.md's file map matches
SCORES nav=8 loc=6 shape=7 self=7 overall=7
Draft F, architect seat
Architect, inherited: comprehension report on @larvit/smpp (draft-f)
1. Map from README and tree only (verbatim, written before opening any source)
Top-level areas I expect, although src/ is flat and shows none of them:
- Public handles:
index.ts,client.ts(SmppClient, reconnecting),server.ts(listener, one Session per connection),session.ts(one socket's life),sms.ts(the inboundsmshandle with sendResp/sendDlr). - Session machinery:
link-timers(enquire_link and idle),reconnect-loop(backoff),send-window(maxOutstanding),pending-requests(seqNr correlation and timeout),outgoing-requests(window plus pending),incoming-requests(dispatch of peer requests),running-handlers(onSms handlers in flight, "Handlers still running" in the README),idle-waiters(maybe the idle timeout?),pdu-transportandpdu-framer(socket to PDUs). - Codec:
pdu.ts,pdu-refusal(PduRefusedError),retained-pdu(a PDU kept for retry?), anddefs/for the spec tables and wire types. - Message content:
message.ts(encode, split, bitCount, smppTime),message-body(short_message vs message_payload),concatandudh(probably the reading and writing of concatenation),reassembly,expiring-groups(the reassembly store?),send-sms(submit composition). - Receipts:
dlr.ts,dlr-merger(messageDlr),sms-id(notations,<base>-<n>). - Plumbing:
result,log,error-from,uuid,defaults,session-options,unanswered-error(went out, no answer),link-lost-error(never reached the socket).
Names that do not give their purpose: idle-waiters, retained-pdu, expiring-groups, error-from, and defaults vs session-options.
What the README led me to expect: a store interface (goal 9). The README itself says it has not shipped, so its absence is fine.
Where the map was wrong, and what each correction cost
idle-waitersis a primitive that waits for a count to fall to zero. It is not the idle timeout, and it also exportsleftOf(), the deadline arithmeticclient.tsuses. Cost: low. The misleading part isleftOfliving there.retained-pducopies a PDU off the wire so holding it does not pin the chunk, and weighs what holding it costs. It has nothing to do with retry. Cost: low. The copy invariant is split withdefs/tlvs.ts:250, which already copies TLVs, soretained-pdu.ts:5's claim that "wire reads hand back views" is only half true.expiring-groupsis a generic capped, weighed, expiring map. Besides reassembly it also backsDlrMerger(twice) and, surprisingly,RunningHandlersas a counter (ExpiringGroups<true>keyed by serial). Cost: medium. "Groups" misleads for the handler counter.concat/udh: splitting is inmessage.ts.udh.tsholds the outgoingConcatReferencecounter plus the parseconcatInfo.concat.tschooses between UDH andsar_*. Cost: medium. It took three files to place the concatenation concepts.session-optionsis not only options. It also holds bind-direction policy (bindCarries,standsInFor),SessionEvents, the hook types, and validation for client- and server-only options (authenticate,connectTimeout,reconnect,fromStart). Cost: medium. I would never have looked there for "which way does a data_sm travel".pdu.tsdepends onmessage.ts(encodeBody,decodeMessage), which depends onudh.ts. So the codec sits above the message layer, not just abovedefs/. Cost: low, but the layering in the AGENTS text is incomplete.- The rest of the map held.
Fan-out, level by level
- L0, the repo:
src,test,docs,benchmarks,interop-tests, plus about 8 top-level.mdfiles. Fine. - L1,
src/: 36 files plusdefs/, so 37 entries. This is the worst level. About six real areas exist, but the layout shows none of them. The only map is the Architecture block in AGENTS.md. - L2,
defs/: 7 files, clean. - L3, the big units:
Sessioncomposes 4 collaborators plus a hand-builtSessionPortof 12 members.IncomingRequestsholdsReassembler,RunningHandlers,createSms, the port and the hooks, and imports 23 symbols.SmppClientholdsReconnectLoop,DlrMergerandConcatReference, plus about 200 lines of free connect and bind functions.
Names
Names that mislead
IncomingRequests.drain(incoming-requests.ts:147-153) calls the count of running handlersunansweredand reports "Shut down with N message(s) unanswered". A handler that already calledsendResp()is still counted. That is the 3am bug's own error text pointing the operator at the wrong thing.RunningHandlers'onDroplogs "giving up on a handler that never returned" for any drop, including anevictedone. Eviction can only be avoided becauserefusedAtBoundis checked first, somewhere else (running-handlers.ts:28).session-options.ts, as above.decodeSegmentslives inreassembly.tsbut is used bysms.ts.ExpiringGroupsused as a counter.
One name over several concepts
- refuse:
Session.refuse(codec-refused PDU),IncomingRequests.refuse(application did not take the message),refusedAtBound,Refusal(a reassembly slot),refusedSegmentStatus,refusalAnswer,PduRefusedError. - end:
Session.end,OutgoingRequests.endandIncomingRequests.endall mean "the socket is gone".SessionPort.endmeans "the peer unbound, destroy the socket".SmppClient.endmeans "the client is over". - answer:
SessionPort.answerwrites a response.sms.ts'sAnsweris mutable answered-state.answerOfconverts the handler's return. - closed: a public getter on
Session, a private field onSmppClient, and a port function.
One concept with two names
- Session lifecycle:
over/closed, andstopping/ "shutting down". - The UDH indicator is checked through
hasUdhin 5 places, and the body is sometimesparams.short_message(a string, or a Buffer when a UDH is present) and sometimesshortMessageOctets. - The submit bind check is spelled twice:
client.ts:159readsoptions.bindType,session.ts:195callsbindAllows.
Restructure, ranked
- Split
IncomingRequests. Routing (route,unhandled,onDelivery) is one piece. Message intake (bound refusal, reassembly, answer-on-arrival,handOver,refuse) is a second, called something likeMessageIntake. It is the densest unit and the one that grows. - Carve bind-direction policy out of
session-options.tsintobind-direction.ts, and move client/server option validation next to its owners or into anoption-checks.ts. - Make the "drain waits on" counter say what it counts. Either release it on
sendResp()plus pending receipts, or rename the error to "handlers still running". Also stop usingExpiringGroupsas the handler counter. - Put concatenation in one module:
ConcatReference,concatInfo,concatOf,udhLength, and the UDH build insplitMessage. - Group
src/into 4–5 folders: handles, link, codec, message, receipts. The flat 37 files is the main Shape cost. - Extract the emitter guard. The
emitoverride,captureRejectionSymboland 7declarelines are copied three times (Session, SmppClient, SmppServer).
What the structure gets right
- Narrow seams:
SessionPort,SmsDeps,SendSmsDeps, and theReconnectLoopOptionscallbacks. Collaborators do not reach into the Session. - Every unit is small and single-noun (
SendWindow,PendingRequests,LinkTimers,PduFramer). Resultis used everywhere, so control flow reads top-down.- Comments carry spec sections and the peer quirks behind them (Jasmin, CM.com, Kaleyra).
defaults.tsis the single source of numbers.LinkLostErrorvsUnansweredErrorencodes goal 2 in the type.
The 3am question
Time to the right unit, cold: about 5–10 minutes, three hops.
- Hop 1: grep "drain" lands in
session.ts:291,Session.drain. - Hop 2: that calls
this.incoming.drain(timeout, signal)atincoming-requests.ts:146. - Hop 3: that calls
RunningHandlers.idle(running-handlers.ts:63), and I had to find wherestart()anddone()are called:IncomingRequests.handOver,incoming-requests.ts:311-314.
The answer: done() fires when the onSms handler returns, not when sms.sendResp() is called. sendResp (sms.ts:126) never touches RunningHandlers. So a handler that answers early and keeps working holds the drain until shutdownTimeout. That includes a handler awaiting sendDlr() to a slow peer: sendDlr goes through port.request, which bypasses the stopping check, and the outgoing drain then waits on it too.
- Is it a bug? README line 399 documents "Wait … for every onSms handler still running", so it is by design. The
sendRespdocstring ("for a handler that keeps working after the answer") invites exactly this expectation. - Right file and unit:
incoming-requests.ts,handOver, together withrunning-handlers.ts. - What slows the hunt: the reported error, "message(s) unanswered", is false for this peer and costs an extra detour into
sms.ts.
Where it rots first: IncomingRequests.onMessage / handOver. Every new inbound rule lands there (per-PDU rate limiting, the store for half-reassembled messages, new data_sm semantics), and each one adds another port.closed() check and another answer path.
Where the next two features would land
- Goal 9's store would land across
Reassembler,DlrMergerandExpiringGroups.ExpiringGroupsis the obvious seam, but it is shared with the handler counter, which must not be persisted. Separate them first. - A per-PDU rate limit (goal 7) would land in
OutgoingRequests.requestbesideSendWindow. That is a clean place. The inbound side would land inIncomingRequestsagain.
Hardest places, ranked
src/incoming-requests.ts:260-326,IncomingRequests.onMessage/handOver. Bound refusal, reassembly, answer-on-arrival, the handler run, and refusal-or-loss are interleaved withclosed()checks andsms.sendRespside effects.src/reassembly.ts:179-198,Reassembler.weighed/dropped. The transientweighingfield is read inside anonDropcallback to discount the newest segment. That is action at a distance through a callback.src/session.ts:291-310withincoming-requests.ts:146andrunning-handlers.ts:50-75,Session.drain. It orders handlers, then requests, on a shared deadline (timeoutfor the first,leftOf(deadline)for the second), then checksclosed. The meaning of "unanswered" is wrong.src/expiring-groups.ts:111-122,ExpiringGroups.expire. It firesonDropmid-iteration.RunningHandlers.onDrop→settle()→size→expire()re-enters it.src/reconnect-loop.ts:64-153withclient.ts:143-156,ReconnectLoop.adopt/attempt/down/stopand the clientsendretry loop onLinkLostError. The state lives insession,timer,attempting,stopped,upAtandlinks.src/pdu.ts:84-136,resolveShortMessage/resolveBody. The rules for which ofshort_messageormessage_payloadownsdata_coding.src/sms.ts:126-231,sendResp/sendDlrover the shared mutableAnswerobject.
The unit I would least want to modify: IncomingRequests, src/incoming-requests.ts:87-358.
Scores
Intrinsic difficulty, which earns no bonus: moderately high. It is an async request/response protocol with reassembly, reconnect, a drain and two ends of the link.
- Navigation 7. Sits at "Predictable". File names map to concepts well enough that the 3am path took three hops. It is held below 8 by the flat 37-file
src/and by the drain's "unanswered" error text pointing atsendResp. - Locality 6. Between 5 and 7. The narrow ports (
SessionPort,SmsDeps) keep collaborators apart. Hidden coupling holds it down:RunningHandlersevicting live handlers unlessrefusedAtBoundruns first, the reassembler'sweighingside channel, and the drain's ordering and shared deadline. - Shape 6. Between 5 and 7. Units are small and mostly honest. Held down by the flat
src/with no visible areas,session-options.tsas a grab bag, concatenation spread over three files, and the overloaded refuse/end/answer/closed vocabulary. - Self-sufficiency 7. Sits at "Predictable". Most units state their invariant and cite the SMPP section at the site (
message.ts:13,pdu.ts:166,sms-id.ts:58). Held below 8 because the area map and the import direction exist only in the AGENTS.md architecture block, not in the layout. - Overall 6. Capped at 7 by the lowest dimension plus one. It sits at 6 because the unit that grows,
IncomingRequests, is the hardest one to change safely.
SCORES nav=7 loc=6 shape=6 self=7 overall=6