From bc4c6b3721f7af4c002b6df649b3a64e96a1ee4b Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Mon, 28 Sep 2026 11:32:50 +0200 Subject: [PATCH] Correct the doc claims the prose sweep falsified, and file the rest --- AGENTS.md | 7 ++++--- MIGRATION.md | 2 +- docs/decisions.md | 12 ++++++------ src/incoming-requests.ts | 1 - todo.md | 21 +++++++++++++++++++++ 5 files changed, 32 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9ac578e..fff9263 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,7 +51,7 @@ src/ dlr.ts Delivery receipts: text and TLV parsing, receipt status codes dlr-merger.ts DlrMerger: per-segment receipts counted into one MessageDlr error-from.ts An untyped value as error material: errorFrom() an Error, namedValue() a name - expiring-groups.ts ExpiringGroups: the capped, weighed, expiring store both of those share + expiring-groups.ts ExpiringGroups: the capped, weighed, expiring store DlrMerger, HeldMessages and Reassembler share held-messages.ts HeldMessages: capped, expiring messages the application has not answered, one MessageHold each idle-waiters.ts IdleWaiters: waiting for a count to fall to zero, and what is left of a budget incoming-requests.ts Every request the peer sends: messages, receipts, links, unknown commands @@ -82,14 +82,15 @@ src/ constants.ts consts + constsById, and the SMPP version constants encodings.ts GSM 03.38, LATIN1, UCS2, detection, data_coding resolution errors.ts errors + errorsById (ESME_*) + index.ts defs: every table as one group tlvs.ts TLV definitions, tlvsById, the typed read and input shapes, and reading and writing a TLV stream types.ts Wire types: int8/int16/int32/string/cstring/buffer/arrays ``` Imports point one way: `defs` knows nothing above it but `result.ts`, `pdu` uses `defs`, `session` uses `pdu`, and `client`/`server` use `session`. The ways back up are the `Session` handed to -`createSms()` and `IncomingRequests`, and to `OnRequest` and `onConnected` in `session-options.ts`, -all imported as a type only. +`createSms()` and `IncomingRequests`, which call back into it, and to `OnRequest` and `onConnected` +in `session-options.ts`, all imported as a type only. **Parameter order is wire order.** The key order inside `cmds.*.params` is the order the fields are written to and read from the buffer. Never sort those alphabetically — the alphabetical-ordering diff --git a/MIGRATION.md b/MIGRATION.md index 8f47396..0296ede 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1,6 +1,6 @@ # Migrating from larvitsmpp 0.4.0 -`@larvit/smpp` 0.5.0 succeeds [larvitsmpp](https://www.npmjs.com/package/larvitsmpp) 0.4.0. The +`@larvit/smpp` succeeds [larvitsmpp](https://www.npmjs.com/package/larvitsmpp) 0.4.0. The shape is the same, connect, send, listen for delivery reports, with callbacks replaced by promises. ## API changes diff --git a/docs/decisions.md b/docs/decisions.md index 6c2247e..1cb5f48 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -8,8 +8,8 @@ rule and an index of the titles below. - **`Session` is publicly constructible, which is what makes `SessionOptions` and `ReconnectOptions` public too.** Raised twice as a leak; it is not one. The collaborators `session.ts` delegates to - (`Reassembler`, `PendingRequests`, `SendWindow`, `ReconnectLoop`, `LinkTimers`, `LinkGate`, - `DlrMerger`, `PduTransport`, `submitSms`) stay unpublished so they can be reshaped. + (`IncomingRequests`, `OutgoingRequests`, `ReconnectLoop`, `LinkTimers`, `DlrMerger`, + `PduTransport`, `submitSms`) stay unpublished so they can be reshaped. - **`acceptsOptionalParams()` and `bindAllows()` are predicates, not chokepoints.** The library's own senders consult them; `session.send({ tlvs })` is passed through as written, because silently @@ -98,9 +98,9 @@ rule and an index of the titles below. optional-parameter threshold.** That threshold is fixed at 0x34 by the spec, so an implementation that must declare 5.0 throughout can, without moving it. -- **A peer that declared no version is pre-3.4, and `undefined` means no bind yet.** `acceptBind()` - records what the ESME declared and the client's `bind()` records the `sc_interface_version` the - SMSC answered with; a peer that declared nothing is recorded as `undeclaredInterfaceVersion` (0x00) +- **A peer that declared no version is pre-3.4, and `undefined` means no bind yet.** `bound()` + records what the peer declared, the ESME's `interface_version` or the SMSC's + `sc_interface_version`; a peer that declared nothing is recorded as `undeclaredInterfaceVersion` (0x00) and sent no optional parameters, which is how the spec reads an absent `sc_interface_version`. - **`esm_class` decides what a `deliver_sm` is, and the body is read only when it names nothing.** @@ -510,7 +510,7 @@ rule and an index of the titles below. Maintainer's call, 2026-08-31: without the split, an application that opens a replacement client on `close` ends up holding two binds on one account. `teardown()` picks the event by whether the reconnect loop is still live, and `end()` stops that loop before tearing down, so every deliberate - shutdown emits `close`. A retry that opens a socket and then loses it clears `closed` through + shutdown emits `close`. A retry that opens a socket and then loses it resets `lifecycle` through `attach()`, which is why a second drop emits again. - **An answer belongs to the link the message arrived on; a receipt does not.** Maintainer's call, diff --git a/src/incoming-requests.ts b/src/incoming-requests.ts index 9c3d375..cb8bed0 100644 --- a/src/incoming-requests.ts +++ b/src/incoming-requests.ts @@ -52,7 +52,6 @@ export type IncomingRequestsOptions = { reassemblyTimeout?: number | undefined; /** Past a drain's refusal, for a receipt the drain is itself waiting for. */ sendPastDrain: (input: PduObjectInput) => Promise>; - /** The session these requests arrive on, which answers them and emits what they carry. */ session: Session; smsIdFormat?: SmsIdFormat | undefined; systemId?: string | undefined; diff --git a/todo.md b/todo.md index 3a969f8..8f5b209 100644 --- a/todo.md +++ b/todo.md @@ -204,6 +204,12 @@ and 5. Every seat ranked the session's lifecycle hardest and least wanted to mod - [ ] **Lift Locality to 7, and confirm it with a scoring run.** A run reading 7.0 or above also retires the #30 decision. The sub-items are what the 2026-09-28 run named, most seats first. +- [ ] **Give the link's liveness one owner.** A third run the same day, after #46, read 6, 6, 7 and + 7, Locality 5, 5, 6 and 6; all four seats ranked `drain()`/`end()`/`teardown()`/`retrying()` + in `session.ts` hardest, because whether the link lives is kept in `Session.lifecycle`, + `ReconnectLoop.halted`, `LinkGate.up`/`returning`, `OutgoingRequests.draining` and + `IncomingRequests.linkGeneration`, held in step by statement order and the comment above + `retrying()`. Four seats. ### Correctness @@ -349,6 +355,21 @@ and 5. Every seat ranked the session's lifecycle hardest and least wanted to mod ### Doc claims this review falsified +- [ ] **Log-cap every interop peer and probe Kannel by protocol, as `interop-tests/AGENTS.md` says + every peer is.** `compose.kannel.yaml` and `compose.smscsim.yaml` carry no `logging:` block, + and Kannel's healthcheck is the bare TCP probe that file warns against. From the prose sweep + of #46. + +- [ ] **Cut what the prose sweep of #46 found restated or misplaced.** `docs/decisions.md` entries + of 30–45 lines carrying pre-fix history (133–160, 217–255, 362–402, 404–440, 442–472, + 593–624, 626–662); AGENTS.md's 14-line shared-fixtures bullet, whose tolerated-copies + reasoning is a decision; the defect table rows MIGRATION.md already carries; README's + `error`-event reason (hard rule 3 owns it) and the Audience bullets restating goal 8 and + Install; the `'use strict'` clause in both MIGRATION.md and CHANGELOG.md; the node-smpp + cross-check in MIGRATION.md; the planned work in `interop-tests/AGENTS.md` (an expected + malformed count per peer) and `benchmarks/README.md`. README persona 3 names a store nothing + ships yet — the maintainer's call, since it is the audience. + - [ ] **Make `LinkGate.isUp()`'s doc true or its state match it.** It says a link attached but not yet bound cannot carry a request, while `up` starts `true`, so the first link and a server session are up before any bind. The gate decision in `docs/decisions.md` makes the same claim