diff --git a/AGENTS.md b/AGENTS.md index ee83e0c..3b858a5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,8 +14,7 @@ not for structure or style. The goals, in priority order, live in [README.md](https://gitea.larvit.se/larvit/smpp-js/src/branch/main/README.md#goals) — they say where this library is heading, which an outside -reader judges it by. The README states the audience alongside them. Everything below cites a goal by -number. +reader judges it by. The README states the audience alongside them. ## Hard rules @@ -83,7 +82,7 @@ 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_*) - tlvs.ts TLV definitions, tlvsById, the input shape, and reading and writing a TLV stream + 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 ``` @@ -162,8 +161,8 @@ decision under [The wire](docs/decisions.md#the-wire). - Hard tabs. Alphabetical ordering for keys, imports and lists unless order is logic-significant. Two deliberate exceptions: command parameters are in wire order (above), and the `errors` and TLV tables are ordered by their numeric id so they can be diffed against the spec and gaps stay visible. -- Comments are the exception, not the default — see the root `CLAUDE.md` rules. Do not write file - preambles or restate what the code says. +- Comments are the exception, not the default. Do not write file preambles or restate what the + code says. - Test data uses real randomised UUID v7 values, never `aaaa-0000` placeholders. - Fixtures that encode the wire are shared so no two files can drift on it: `test/raw-pdus.ts` builds the octets a test writes straight to a socket, the PDUs `objToPdu()` refuses to build included. So diff --git a/CHANGELOG.md b/CHANGELOG.md index 334e5b6..b46c614 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,8 +59,8 @@ for the first occurrence. `objToPdu()`, `session.send()` and `session.sendReturn()` take `{ tagValue: [value] }` for them and refuse a lone value before anything goes out. - `pduObj.tlvs` and every `tlvs` input are typed per tag, as `Tlvs` and `TlvInputs`: - `receipted_message_id` reads as a `string`, `callback_num` as a `Buffer[]`, and a value of the wrong - type for its tag fails to compile. Annotate with `Tlvs` or `TlvInputs` where you wrote + `receipted_message_id` reads as a `string`, `message_state` as a `number`, and a value the tag + cannot carry fails to compile. Annotate with `Tlvs` or `TlvInputs` where you wrote `Record` or `Record`; `TlvInput` is gone. **A TLV input is keyed by its name, or by its decimal id where the table names none, and a `tagId` @@ -68,9 +68,11 @@ `{ vendor: { tagId: 5142, … } }`; `{ message_state: { tagId: 5, … } }` used to go out as tag 5. A parsed PDU's `tlvs` still relay as they are. A number for an octet TLV, vendor tags included, is refused, where it went out as its ASCII digits. + A decimal key naming a tag the table knows, `{ 1063: … }`, is refused in favour of the name, and so are `alert_on_msg_delivery` and `failed_broadcast_area_identifier` in favour of - `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under. + `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back under. The + two alternate names are gone from `tlvs` too. - `cmds.broadcast_sm_resp.tlvMap` is removed; nothing read it. ## 0.5.0 diff --git a/MIGRATION.md b/MIGRATION.md index 77d4253..2759f16 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -32,9 +32,10 @@ shape is the same, connect, send, listen for delivery reports, with callbacks re `consts.ESM_CLASS.STORE_FORWARD` reads `undefined`, which OR-s into an `esm_class` carrying no mode. - **A TLV is keyed by its name, or by its decimal id where the table names none**, and a `tagId` disagreeing with its key is refused: `{ 5142: { tagValue } }`, not - `{ vendor: { tagId: 5142, tagValue } }`. A number for an octet TLV is refused; give a Buffer. + `{ vendor: { tagId: 5142, tagValue } }`. A number for an octet TLV is refused; give a Buffer or a string. Write `alert_on_message_delivery` and `broadcast_area_identifier`, the names they read back - under, for `alert_on_msg_delivery` and `failed_broadcast_area_identifier`. + under, for `alert_on_msg_delivery` and `failed_broadcast_area_identifier`, which are gone from + `tlvs` too. - **The `error` event is `sessionError`**, and `serverError` on the server handle. - **`log`** takes any object with `debug`, `error`, `info`, `verbose` and `warn` methods instead of a `larvitutils` one, and is silent by default: [README](README.md#logging). diff --git a/benchmarks/README.md b/benchmarks/README.md index ffc64de..fb224e7 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -59,8 +59,8 @@ Against real peers, same driver, window 50, 20,000 messages: | SMSC | msgs/s | failed | | --- | --- | --- | | this library's sink | 37,125 | 0 | -| Jasmin 0.10 | 2,207 | 0 | -| SMPPSim 3.0.0 | — | 19,000 of 20,000 | +| Jasmin 0.11.0 | 2,207 | 0 | +| SMPPSim 2.6.11 | — | 19,000 of 20,000 | ## Against the other client libraries diff --git a/interop-tests/AGENTS.md b/interop-tests/AGENTS.md index 41a6201..c1476ca 100644 --- a/interop-tests/AGENTS.md +++ b/interop-tests/AGENTS.md @@ -93,7 +93,7 @@ ways the next experiment must see. One fix per defect class, as its own change: 1. A worktree on a branch off `origin/main` (never `origin/v0.4.0`, the 0.4.0 code), named for the defect. 2. Regression tests in `test/` first, naming the behaviour with the reproducer from the findings; - then the implementation; then the decision record in the root `AGENTS.md` where the fix settles + then the implementation; then the decision record in `docs/decisions.md` where the fix settles a question of the wire or the session's life. 3. `/larv-review` on the branch, with the pull request based on `main`. When it marks the PR ready, fast-forward it. diff --git a/interop-tests/README.md b/interop-tests/README.md index ecc1c70..29bfda9 100644 --- a/interop-tests/README.md +++ b/interop-tests/README.md @@ -1,6 +1,6 @@ # interop-tests -Eight real SMPP implementations, run against this library in both directions, with every session +Ten real SMPP implementations, run against this library in both directions, with every session decoded independently by tshark so no result rests on our own view of the wire. It exists because the unit suite and this library's own dummy peers agree with themselves; these peers do not. @@ -40,7 +40,7 @@ These bind to our server: | Peer | What it is for | Run | | --- | --- | --- | -| **Kannel 1.4.5** | The most deployed real ESME there is; parses our receipts with the parser most operators' customers run, and declares 3.4 or 3.3 on demand | `debian:bookworm-slim` + the distribution package; four `.conf` variants under `peers/kannel/` | +| **Kannel 1.4.5** | The most deployed real ESME there is; parses our receipts with the parser most operators' customers run, and declares 3.4 or 3.3 on demand | `debian:bookworm-20260824-slim` + the distribution package; four `.conf` variants under `peers/kannel/` | | **jsmpp** | Strict and low-level: the driver builds UDH, `sar_*` and `message_payload` bytes by hand, and rejects an answer it dislikes | Maven build at a pinned commit, `peers/jsmpp/` | | **Cloudhopper** | The one peer with real windowing knobs, plus a TLS client | Maven build at a pinned commit, `peers/cloudhopper/`. Its 2015-era TLS client cannot do 1.3, so that scenario caps the server at 1.2 | | **python-smpplib 2.2.4** | An independent GSM 03.38 table to cross-check ours character by character | `python:3.12.14-slim-bookworm`, `peers/python/` | diff --git a/src/defs/tlvs.ts b/src/defs/tlvs.ts index d02ef23..85425e8 100644 --- a/src/defs/tlvs.ts +++ b/src/defs/tlvs.ts @@ -83,13 +83,13 @@ type Specs = typeof specs; export type TlvName = keyof Specs; -// Alternate spellings; the definition behind each keeps its canonical name. -const alternates = { - alert_on_msg_delivery: specs.alert_on_message_delivery, - failed_broadcast_area_identifier: specs.broadcast_area_identifier, +// SMPP 5.0's other spellings, which only name the tag to key instead. +const alternates: Record = { + alert_on_msg_delivery: 'alert_on_message_delivery', + failed_broadcast_area_identifier: 'broadcast_area_identifier', }; -export const tlvs: Record & Record = { ...specs, ...alternates }; +export const tlvs: Record & Record = specs; export const tlvsById: Record = {}; @@ -114,7 +114,6 @@ type WriteValue = Specs[K] extends { multiple: true } ? ReadV type KnownTlv = { tagId: number; tagName: K; tagValue: ReadValue }; -/** A tag the TLV table does not define. */ type UnknownTlv = { tagId: number; tagName: undefined; tagValue: Buffer }; /** Keyed by tag name, or by its decimal id where the table defines no name. */ @@ -122,7 +121,7 @@ export type Tlvs = { [K in TlvName]?: KnownTlv } & Partial }[TlvName] | UnknownTlv; -/** Keyed like `Tlvs`, by tag name or by the decimal id of a tag the table does not define. */ +/** Keyed like `Tlvs`. */ export type TlvInputs = { [K in TlvName]?: { tagValue: WriteValue } } & Partial>; @@ -139,9 +138,9 @@ function isTlvInput(input: unknown): input is { tagValue: TlvValue } { function keyedTagId(name: string): Result<{ tagId: number }> { if (isTlvName(name)) return { tagId: specs[name].id }; - const alternate = Object.hasOwn(tlvs, name) ? tlvs[name] : undefined; + const alternate = Object.hasOwn(alternates, name) ? alternates[name] : undefined; - if (alternate) return { err: new Error(`TLV "${name}": key it ${alternate.tag}, the name it reads back under`) }; + if (alternate) return { err: new Error(`TLV "${name}": key it ${alternate}, the name it reads back under`) }; if (!/^(0|[1-9]\d*)$/.test(name)) { return { err: new Error(`TLV "${name}": unknown tag name; key a tag the table does not define by its decimal id`) }; @@ -156,7 +155,6 @@ function keyedTagId(name: string): Result<{ tagId: number }> { return known ? { err: new Error(`TLV "${name}": the table names this tag ${known.tag}, key it by that`) } : { tagId }; } -/** The key names the tag; a `tagId` beside it, as a parsed TLV carries, has to agree. */ function entryOf(name: string, input: unknown): Result<{ tagId: number; tagValue: TlvValue }> { if (!isTlvInput(input)) { return { err: new Error(`TLV "${name}": give it as { tagValue }, holding a Buffer, a number, a string, or an array of Buffers or of numbers`) }; diff --git a/todo.md b/todo.md index 2a11169..79cc2bb 100644 --- a/todo.md +++ b/todo.md @@ -194,10 +194,6 @@ and is also what the panel ranked hardest — two methods, one answer. rest silently, which goal 3 argues against; listing it would change every such tag's shape. From the architecture review of #25. -- [ ] **Refuse a TLV input naming one tag under both its spellings.** `broadcast_area_identifier` - and `failed_broadcast_area_identifier` in one `tlvs` record both write, so the peer receives - the union of two lists the caller may have meant as one. From the architecture review of #25. - - [ ] **Test that a multipart send which errors never fires `messageDlr`.** Goal 2 now says so and README promises it; `session-extras.test.ts` covers a drop *after* the send, not one during it. @@ -397,6 +393,15 @@ and is also what the panel ranked hardest — two methods, one answer. ## Worth doing, not blocking +- [ ] **Move the decisions out of AGENTS.md's fixtures paragraph and the two tooling READMEs.** The + one dummy SMSC, `smscPeer()` staying separate and which copied helpers are tolerated + (AGENTS.md Conventions), and why Kannel is absent from `benchmarks/README.md`, go to + `docs/decisions.md` with index lines. Move the planned work written into + `interop-tests/AGENTS.md` (an expected count per peer) and `benchmarks/README.md` (the default + window gap) to this file. Give the `run.py`-in-background footgun in `interop-tests/AGENTS.md` + a rule of its own. Delete AGENTS.md's "message_id values … are UUID v7" line. From the prose + pass of #30. + - [ ] **Make the dumbclient soak's memory sample evidence of no library leak again.** Its rss ends at its maximum (298 MiB, heapUsed 81 MiB after 173,820 messages), which the harness's own per-id `Set` and `answerOrder` explain but cannot separate from a leak in `src/`: sample the heap