diff --git a/AGENTS.md b/AGENTS.md index 68fbd95..86266bf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ not for structure or style. ## Goals -The ten goals, in priority order, live in +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. @@ -86,8 +86,9 @@ src/ types.ts Wire types: int8/int16/int32/string/cstring/buffer/arrays ``` -Dependency direction is one way: `defs` knows nothing above it, `pdu` uses `defs`, `session` uses -`pdu`, and `client`/`server` use `session`. Nothing reaches back up. +Imports point one way: `defs` knows nothing above it but `result.ts`, `pdu` uses `defs`, `session` +uses `pdu`, and `client`/`server` use `session`. The one way back up is the `Session` handed to +`IncomingRequests` and `createSms()`, 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 @@ -103,20 +104,12 @@ docker compose run --rm node npm test docker compose run --rm node npm run build ``` -- Tests are `.ts` and run directly under Node's type stripping — no build step in the dev loop. - Source imports use `.ts` extensions; `rewriteRelativeImportExtensions` emits `.js` into `dist`. - `erasableSyntaxOnly` is on, so no enums, no namespaces, no parameter properties. Use `as const` objects plus union types. -- The published floor is Node 18, but the dev container runs Node 24 (type stripping needs it). CI - compiles the tests and runs them on 18, every LTS above it, and current, so the floor is - verified rather than asserted. +- The published floor is Node 18; the dev container runs Node 24 because type stripping needs it. - `typescript` is pinned to the 6.x line because `typescript-eslint` peer-requires `<6.1.0`. Move to TypeScript 7 once that constraint lifts. -- GitHub mirrors Gitea through `.gitea/workflows/mirror.yaml`, which never prunes, and - `mirror-delete.yaml`, one run per deleted ref. A delete run that fails or outlives Gitea's queue - timeout, or a push run that cloned before the delete, leaves the ref on GitHub until the delete - run is re-run. Accepted: a stale ref there is harmless, and refs only GitHub has must survive. - Maintainer's call, 2026-09-14; valid while nothing deploys from GitHub. ## Defects found in 0.4.0 @@ -152,13 +145,6 @@ Confirmed by reading the 0.4.0 source; each row has a regression test naming the | `ESME_RINVBCASTCHANIND` typo | Defined as `0x011`, three hex digits; the spec value is `0x0112` | | Every response carries a message id | `session.js` builds `params = {'message_id': …}` for every response it sends, `deliver_sm_resp` included; SMPP 3.4 4.6.2 makes that field unused and NULL, and Jasmin closes the connection on one | -## Multipart sends - -`sendSms` puts every segment of a message on the wire together instead of waiting for each response -in turn, so a long message costs one round trip rather than one per segment. Nothing on the -receiving side forces the order either way: this library answers each inbound segment as it arrives, -so a peer that dispatches one request at a time is never left waiting on us. - ## GSM 7-bit is sent unpacked Over SMPP the ESME puts one GSM character per octet in `short_message` and the SMSC packs it into @@ -249,7 +235,6 @@ the file. - `Session` is publicly constructible, which is what makes `SessionOptions` and `ReconnectOptions` public too. - `acceptsOptionalParams()` and `bindAllows()` are predicates, not chokepoints. -- `session.sock` is a getter over `PduTransport`. - Both emitters re-declare their listener methods to accept a promise. - `PduRefusedError` is exported, and `sessionError` names it in the event's type. - `bitCount()`, `encodeMessage()` and `splitMessage()` keep their total signatures, because @@ -308,6 +293,7 @@ the file. `false` is the one way to turn it off. - A stream this library cannot frame is a dead link; one PDU it cannot parse is not. - A deliberate shutdown drains; an unusable link and an abort do not. +- `sendSms()` puts every segment of a message on the wire together. - Every segment of a concatenated message is answered as it arrives, so `sendResp()` on one is the application's own signal rather than the peer's answer. - `server()` composes the application's `onRequest` after its own bind handling, and offers it every @@ -331,3 +317,5 @@ the file. - `test/` stays flat too, and a file there is named for the question it answers rather than for the module it covers. - CI tests on Linux only; `src/` keeps off what is known to break on macOS or Windows. +- GitHub mirrors Gitea without pruning, and a ref deleted on Gitea is deleted on GitHub by a run of + its own. diff --git a/README.md b/README.md index 4395005..d93ca5e 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,6 @@ await smpp.close(); // stop listening, then drain and close every live sess `sendDlr('UNDELIVERABLE')` any other state: [Server in depth](#server-in-depth). - A message that arrived in several segments was answered as they arrived, so `sendResp()` there takes no `smsId` or refusing `status`. `sms.answeredOnArrival` says which case you are in. -- `smpp.close()` stops listening, then drains and closes every live session. ## Errors @@ -262,7 +261,7 @@ All optional. Timeouts are milliseconds. | `tls` | `false` | A `tls.TlsOptions` object with your certificate and key. A bare `true` is refused. | | `idleTimeout` | `40000` | Drop a peer that has been silent this long. | | `maxReassembly` | `1000` | Incomplete multipart messages held per session. | -| `maxOctets` | `67108864` | Bytes of incomplete multipart messages held per session. | +| `maxOctets` | `67108864` | Roughly the memory incomplete multipart messages may hold per session. | | `reassemblyTimeout` | `300000` | How long a late segment can still join an incomplete message. | | `responseTimeout`, `shutdownTimeout`, `maxOutstanding`, `log`, `signal` | as for the client | | @@ -701,7 +700,7 @@ one wins. They do not override the [hard rules](https://gitea.larvit.se/larvit/s to the application to persist. Coordinating processes any other way is declined without a fresh argument each time. 10. **It builds, tests and runs the same everywhere.** Container-only toolchain, no runtime - dependencies, the Node 18 floor verified in CI rather than asserted, every README example executed + dependencies, the Node 18 floor verified in CI, every README example executed by the suite. ## Audience @@ -710,8 +709,7 @@ Who depends on this library, and what they may rely on. - **The public npm audience.** Only what `src/index.ts` exports is public; everything behind it is reshaped freely. -- **Node 18 and newer, ESM only, no runtime dependencies.** The floor is verified in CI rather than - asserted, so the library drops into a service or a container without pulling a tree behind it. +- **Node 18 and newer, ESM only, no runtime dependencies.** - **Real SMSCs and ESMEs as operators actually run them**, not a reference implementation. Jasmin, SMPPSim, Kannel, jsmpp, Cloudhopper, python-smpplib and php-smpp are the interop targets, and what they do in practice outranks what the specification says they should do. diff --git a/docs/decisions.md b/docs/decisions.md index b3b7e33..5034230 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -17,9 +17,6 @@ rule and an index of the titles below. sending them. Only `submit_sm`, `deliver_sm` and `data_sm` are policed by bind direction — the three the library dispatches by it, of which it sends the first two. -- **`session.sock` is a getter over `PduTransport`.** Reading it is unchanged; assigning it no longer - compiles, which never rewired the handlers and so never worked. - - **Both emitters re-declare their listener methods to accept a promise.** Maintainer's call, 2026-08-27: `EventEmitter` types every listener as void-returning, so the `session.on('sms', async sms => …)` README documents reads as a misused promise in any strict @@ -133,8 +130,7 @@ rule and an index of the titles below. rule purely additive: no PDU that parsed before reads differently now. Rejected: preferring the TLV, which re-reads every message a peer echoes into both. Rejected: refusing a PDU carrying both, which discards a message that is almost certainly present twice over, where goal 3 keeps the - traffic. The reassembler's octet cap already counts TLV values, so a 64 KB payload is bounded like - any other segment. + traffic. - **A segment's concatenation is read from its UDH, or from the `sar_*` TLVs where it declares none, and each spelling groups in a reference space of its own.** Maintainer's call, 2026-09-06, from @@ -359,8 +355,8 @@ rule and an index of the titles below. it. There is one budget, 140 less the UDH, and the alphabet decides only what it is counted in, so Latin-1 and UCS2 both take those 134 octets — 134 characters and 67 — and it is GSM 7-bit's 153 that is the odd number rather than the other way round. `Record` is what makes - a fourth alphabet state its own. Rejected: 134 for GSM 7-bit too, which is the mistake the - unpacked-alphabet section above exists to stop. Accepted: a Latin-1 message past the 140 characters + a fourth alphabet state its own. Rejected: 134 for GSM 7-bit too, which is the mistake + [GSM 7-bit is sent unpacked](../AGENTS.md#gsm-7-bit-is-sent-unpacked) exists to stop. Accepted: a Latin-1 message past the 140 characters one SMS holds now costs more segments than it did, and `smsIds` is that much longer. - **An alphabet the caller named has to carry the message, and a time the format cannot express is @@ -588,6 +584,10 @@ rule and an index of the titles below. reports each session's unfinished drain through `serverError`, because its own result says nothing but that the listener stopped. +- **`sendSms()` puts every segment of a message on the wire together.** Goal 6: a long message costs + one round trip rather than one per segment. Rejected: sending each segment once the last is + answered, which a receiver waiting for the whole message before answering would deadlock. + - **Every segment of a concatenated message is answered as it arrives, so `sendResp()` on one is the application's own signal rather than the peer's answer.** Maintainer's call, 2026-09-06, from the Jasmin interoperability phase: Jasmin dispatches one `submit_sm` per connector at a time and will @@ -789,7 +789,7 @@ rule and an index of the titles below. dev image has no openssl. - **`src/` stays flat until a module has to move for another reason.** Architecture review, - 2026-09-06: the grouping the file map above already implies — `wire/` for `pdu*` and `defs`, + 2026-09-06: the grouping the [file map](../AGENTS.md#architecture) already implies — `wire/` for `pdu*` and `defs`, `link/` for `link-*`, `reconnect-*`, `pdu-transport` and `send-window`, `messages/` for `sms*`, `dlr*`, `message*`, `reassembly` and `udh` — rewrites every import for no change to `dist/index.js`, the one published entry. Valid while that map is what a reader navigates by. @@ -809,3 +809,10 @@ rule and an index of the titles below. That binds what `dist/` runs; the container tooling, `interop-tests/` and the `package.json` scripts run on Linux by goal 10. Rejected: macOS and Windows runners, on GitHub's mirror or as Gitea host-mode runners on a Windows VM and a Mac. + +- **GitHub mirrors Gitea without pruning, and a ref deleted on Gitea is deleted on GitHub by a run of + its own.** Maintainer's call, 2026-09-14; valid while nothing deploys from GitHub. + `.gitea/workflows/mirror.yaml` never prunes, and `mirror-delete.yaml` runs once per deleted ref. A + delete run that fails or outlives Gitea's queue timeout, or a push run that cloned before the + delete, leaves the ref on GitHub until the delete run is re-run. Accepted: a stale ref there is + harmless, and refs only GitHub has must survive. diff --git a/todo.md b/todo.md index 5bb8c55..7b83a5f 100644 --- a/todo.md +++ b/todo.md @@ -262,8 +262,8 @@ and is also what the panel ranked hardest — two methods, one answer. - [ ] **Give `IncomingRequests` a port instead of the `Session` it drives.** It holds its owner and calls eight members of it 18 times, including `this.session.close()` on an inbound `unbind` — a collaborator ending its owner's life. `OutgoingRequests` is the mirror half of the same - boundary and takes no session at all. AGENTS.md's "Nothing reaches back up" is false because of - this, and `docs/decisions.md` already states the rule under The session's life: "a collaborator + boundary and takes no session at all. AGENTS.md names this as the one way back up; the port + removes that exception, and `docs/decisions.md` already states the rule under The session's life: "a collaborator that has to ask does not own its decision". It is also the missing test seam — inbound routing, reassembly dispatch, `onRequest` ordering and bind-direction refusal have no unit test because the class cannot be built without a live socket. Carry the eight members as `IncomingDeps`, @@ -399,10 +399,6 @@ and is also what the panel ranked hardest — two methods, one answer. the fenced blocks at test time and assert each appears verbatim in the executed source, so an edit to either fails the gate. -- [ ] **Correct AGENTS.md's "Nothing reaches back up".** False while `IncomingRequests` holds a - `Session`: either the first Locality item makes it true, or the sentence names the exception - until it does. - - [ ] **Narrow the `src/defs/*` lint exemption to the four table files.** Its stated reason — "the spec tables are data: their length tracks the specification, not any complexity" — is false for `defs/types.ts`, which is 595 lines of wire codec with 25 functions and is the file that parses