Files
smpp-js/todo.md
T
lilleman ef13290230 Refuse a body the PDU's own data_coding cannot carry (#98)
* Regression tests for a body the PDU's own data_coding cannot carry

* Refuse a body the PDU's own data_coding cannot carry, in the codec both send paths build through

* Record the codec's body refusal, and drop two todo entries it and the interop run close

* Regression tests for the aliased body TLV and the data_coding short_message settles

* Find the body TLV by tag id, leave data_coding to the field that will be read, and correct the record

* Record the two architecture findings this change does not close

* Regression tests for a duplicated body tag and a short_message the wire never carries

* Resolve every body TLV against the field that will be read, and move TLV writing to its table

* Regression tests for a body only the TLV carries and an empty short_message

* Let only octets the command writes settle the alphabet, and stop shadowing the TLV table
2026-09-09 19:33:36 +02:00

227 lines
16 KiB
Markdown

# todo.md
Remaining work for the `@larvit/smpp` 1.0.0 rewrite. Read [AGENTS.md](AGENTS.md) first — the goals
and hard rules there constrain every item below.
This is a temporary working file: it is deleted when 1.0.0 ships, and until then it sets its own
rules. The documentation conventions in AGENTS.md do not govern it, and nothing here is a source
anything else may cite.
## Status
The rewrite is **feature complete and green**: the suite, lint and typecheck are clean on Node 18,
20, 22 and 24. What is left is release work and a few things worth adding before or after 1.0.0.
## The agreed API
Settled with the maintainer before implementation. Do not change any of it without asking. The
public surface is documented in [README.md](README.md); this is the short form.
```ts
import { client, server } from '@larvit/smpp';
const { err, session } = await client({ host, password, port, username });
const { err: sendErr, pduObjs, smsIds } = await session.sendSms({ dlr, from, message, to });
await session.unbind();
const { err: serverErr, server: smpp } = await server({ authenticate, port });
smpp.on('session', session => {
session.on('sms', async sms => {
await sms.sendResp();
if (sms.dlr) await sms.sendDlr('DELIVERED');
});
});
await smpp.close();
```
Rules the API follows:
- **Never throws.** Everything fallible resolves to `{ err?, … }`. See AGENTS.md rule 1.
- **Named exports only**, no default export. `defs` is exported as a group alongside the individual
tables.
- **The PDU codec is synchronous** and returns `{ err?, pduObj? }` / `{ err?, buffer? }`.
- **Low-level surface stays public**, including `session.sock`, `session.send()` and
`session.sendReturn()`.
## Done
| | Covered by |
| --- | --- |
| Definition tables: constants, errors, encodings, wire types, TLVs, commands | `test/encodings.test.ts`, `test/types.test.ts`, `test/commands.test.ts` |
| Message helpers: splitting, bit counting, SMPP dates and times | `test/message.test.ts` |
| PDU codec: parse, build, respond, per-command typing, bounds checks | `test/pdu.test.ts` |
| Stream framing | `test/pdu-framer.test.ts` |
| Delivery receipt parsing, TLV and text | `test/dlr.test.ts` |
| Session, client, server: bind, auth, send, reassembly, DLRs, timeouts, abort, send window | `test/session.test.ts` |
| Merged multipart DLRs including across a reconnect, reassembly bounds, per-send abort, the segment cap | `test/session-extras.test.ts` |
| `smsIdFormat`: a peer's `submit_sm_resp` and receipt ids read into one notation before they are compared | `test/dlr.test.ts`, `test/session-extras.test.ts` |
| A draining `close()` and `unbind()`, bounded by `shutdownTimeout` or an abort | `test/session-extras.test.ts` |
| A drain that also waits out the messages the application has not answered, with `sendDlr()` the one send that passes it | `test/session-extras.test.ts` |
| `OutgoingRequests`: the gate, the window, the pending map and the retry under one owner, told when a link comes up or goes down | `test/session-extras.test.ts`, `test/session.test.ts` |
| Held messages capped and expiring, so an application that answers nothing cannot grow them | `test/session-extras.test.ts` |
| A send with no link held for the next one, and one the link dropped under counted as `unanswered` | `test/session-extras.test.ts` |
| A message whose link dropped refused an answer, with its receipt still allowed out | `test/session-extras.test.ts` |
| The hold released exactly when the peer was answered: a refused `sendResp()` keeps it, a listener that rejected drops it | `test/session-extras.test.ts` |
| Every runnable README example | `test/readme.test.ts` |
| Receipt-versus-message classification by `esm_class` | `test/dlr.test.ts`, `test/session.test.ts` |
| An intermediate delivery notification read as a report marked `intermediate`, as is a receipt reporting `ENROUTE` or `SCHEDULED`, and never counted into a merge | `test/dlr.test.ts`, `test/session.test.ts`, `test/session-extras.test.ts` |
| A transient state sent under the marker the spec gives it, off the same list the reader uses | `test/session-extras.test.ts` |
| A listener that throws, or rejects, reaching `sessionError`/`serverError` rather than the process | `test/session.test.ts`, `test/error-from.test.ts` |
| Cross-checked against node-smpp both ways and over a live session | `test/interop.test.ts` |
| CI on Node 18/20/22/24, Renovate, tag-triggered publish | `.github/workflows/` |
Every defect listed in the AGENTS.md table has a regression test naming the behaviour.
## The GitHub backlog, once this branch is `main`
Nothing below is closed while the default branch is still 0.4.0 — declining a security bump on a live
default branch is worse than leaving it open. Work through this immediately after the rename.
**Close as fixed by 1.0.0**, naming the replacement in the comment:
| | Fixed by |
| --- | --- |
| [#4](https://github.com/larvit/larvitsmpp/issues/4) DLR errors with `message_state` missing | `dlrFromPdu()` parses the `stat:` receipt text when the TLVs are absent |
| [#33](https://github.com/larvit/larvitsmpp/issues/33) Large inbound text arrives as raw `Buffer` segments | `IncomingRequests` reassembles a UDH-carrying `deliver_sm` into one `sms` event |
| [#3](https://github.com/larvit/larvitsmpp/issues/3) Tests for flash messages | `test/session.test.ts` |
| [#20](https://github.com/larvit/larvitsmpp/issues/20) Tests fail on current dependency versions | The mocha suite is gone; `node:test` on Node 18/20/22/24 |
| [#2](https://github.com/larvit/larvitsmpp/issues/2) Tests for the README examples | `test/readme.test.ts` |
| [#17](https://github.com/larvit/larvitsmpp/issues/17) `addr_ton`/`addr_npi` should be settable | `sendSms()` takes all four, documented and tested |
| [#16](https://github.com/larvit/larvitsmpp/issues/16) Support all three bind types | Bound and enforced in both directions |
| [#13](https://github.com/larvit/larvitsmpp/issues/13) Limit a long SMS to fewer segments | The `maxSegments` send option |
| [#68](https://github.com/larvit/larvitsmpp/pull/68) `message_id` in `submit_sm_resp`, spec DLR codes | All four hold: `sendResp()` always answers a `message_id`, per segment; `stat:UNDELIV` is the 7-character code. Credit the reporter — the fork found real defects. |
**Close as superseded**, all against 0.4.0 dependencies the rewrite does not have — `async`,
`coveralls`, `eslint`, `iconv-lite`, `larvitutils`, `mocha`, `mocha-eslint`, `portfinder`, `uuid`:
[#40](https://github.com/larvit/larvitsmpp/pull/40), [#41](https://github.com/larvit/larvitsmpp/pull/41),
[#42](https://github.com/larvit/larvitsmpp/pull/42), [#45](https://github.com/larvit/larvitsmpp/pull/45),
[#46](https://github.com/larvit/larvitsmpp/pull/46), [#47](https://github.com/larvit/larvitsmpp/pull/47),
[#59](https://github.com/larvit/larvitsmpp/pull/59), [#63](https://github.com/larvit/larvitsmpp/pull/63),
[#64](https://github.com/larvit/larvitsmpp/pull/64), [#65](https://github.com/larvit/larvitsmpp/pull/65),
[#67](https://github.com/larvit/larvitsmpp/pull/67), [#70](https://github.com/larvit/larvitsmpp/pull/70).
[#70](https://github.com/larvit/larvitsmpp/pull/70) is the open `uuid` advisory GitHub reports on the
default branch; it disappears with the runtime dependencies rather than being fixed.
[#60](https://github.com/larvit/larvitsmpp/issues/60) is Renovate's dashboard — leave it, it
re-baselines itself against the new `package.json`.
**Leave open:** [#8](https://github.com/larvit/larvitsmpp/issues/8), the socket's remote host and
port on log messages. Only `server - incoming connection` carries them today; putting them on every
session message is a change to every call site.
## Before publishing 1.0.0
- [ ] Create the `@larvit/smpp` package on npm and add `NPM_TOKEN` to the repository secrets, which
`.github/workflows/release.yaml` needs.
- [ ] Decide whether `smsIds` should be `(string | undefined)[]`. An SMSC that names an id for the
first segment of a concatenated message only — Telesign documents exactly that — leaves the
rest empty, so `string[]` promises what the value does not keep: taking the first id, or
building a map from the array, compiles and then misbehaves, and an empty string collides with
another message's in a correlation table. The cost is that every consumer narrows, including
the majority whose SMSC always names an id. Raised by the phase 11 product review, 2026-09-08.
**This one is 1.0.0-or-never** — after release it needs a major version.
- [ ] Tag `v1.0.0` to publish.
- [ ] `npm deprecate larvitsmpp` pointing at `@larvit/smpp`. Maintainer's call to run it; not
something CI should do.
- [ ] **Rename the branches, once everything above is done.** Maintainer's call, 2026-09-04:
`master` becomes `v0.4.0`, `typescript` becomes `main`, and `main` is the repository's default
branch. Renaming rather than merging is what the orphan commit leaves available — the two
histories share no ancestor, so a merge refuses them outright.
- [ ] Rename `master` to `v0.4.0`.
- [ ] Rename `typescript` to `main`, and make it the default branch.
- [ ] Retarget what still points at an old name: [#71](https://github.com/larvit/larvitsmpp/pull/71),
and the open PRs based on `master`, which a rename carries over rather than closes.
- [ ] Delete `rewrite-base` once [#71](https://github.com/larvit/larvitsmpp/pull/71) is
resolved; it exists only to give that PR a reviewable diff.
## Worth doing, not blocking
- [ ] **A send the codec will refuse waits for a link and a window slot first.** `refuse()` in
`outgoing-requests.ts` runs `misuse()` and the abort check before the wait, precisely so a call
that can never go out does not queue for what it will never use; a body `objToPdu()` refuses on
every attempt is the same case, and #98 made it a common one. On a down link the caller waits
`responseTimeout` and is told the link failed rather than that the body could not be built —
goal 2's wrong answer about what happened. The cheap fix builds the PDU twice, so the shape is
the open half. Raised by the architecture review of
[#98](https://github.com/larvit/larvitsmpp/pull/98), 2026-09-09.
- [ ] **`message.ts` answers two questions.** Message coding and the SMPP time format (`smppDate`,
`smppTime`) share the file, which the architecture map in AGENTS.md already spells out as four
concerns. Nothing is wrong today; if the file has to move for another reason, `smpp-time.ts` is
the split. Raised by the architecture review of
[#98](https://github.com/larvit/larvitsmpp/pull/98), 2026-09-09.
- [ ] **A gate that refuses a floating version anywhere in the repo.** Maintainer's ask on
[#71](https://github.com/larvit/larvitsmpp/pull/71), 2026-09-06, on the `release.yaml`
pinning thread, which stays open until this lands. Pinning every action and runner by hand is
what the ask followed; the gate is what keeps them pinned. It has to cover workflow `uses:`
and `runs-on:`, compose `image:`, and Dockerfile `FROM`, and the conventions differ per kind —
actions take a semver tag, images the full patch version — so one grep for `latest` is not it.
- [ ] **A gate that fails when the test matrix misses the current Node.** Maintainer's ask on
[#71](https://github.com/larvit/larvitsmpp/pull/71), 2026-09-06, on the Node 26 thread, which
stays open until this lands. Node 26 was added by hand; nothing notices when 27 ships. Needs a
source for what Current is — the Node release schedule is published as JSON — and a decision
on whether a new Current fails the build or opens a PR, which is what Renovate already does
for everything else here.
- [ ] **Group the session's collaborators under `src/session/`.** `session.ts` imports
`dlr-merger`, `incoming-requests`, `link-timers`, `outgoing-requests`, `pdu-transport`,
`reconnect-loop` and `send-sms`, and nothing else does, so the directory would make that
boundary visible. The `OutgoingRequests` extraction this was to be done with landed on
2026-09-01, so it is the remaining half. Raised by review, 2026-09-01.
- [ ] **`leftOf()` and the link gate's own budget are one concept counted twice.**
`idle-waiters.ts` reads what is left of a budget as `Math.max(1, deadline - now)`, because 0
means "forever" there; `link-gate.ts` runs the same subtraction and calls `<= 0` expired.
Neither is reachable from the other, so nothing can disagree today, but a reader who learns one
and applies it to the other is wrong. A budget type both take would close it. Raised by review,
2026-09-01.
- [ ] **`err:` on a receipt for a state that neither delivered nor failed.** `receiptText()` now
writes `err:000` for `DELIVERED` and for the two transient states, and `err:001` for every
other — so `ACCEPTED`, `SKIPPED`, `UNKNOWN` and `DELETED` still announce an error code the SMSC
never had. Which of those are failures is the open half. Raised by review, 2026-09-03; needs a
decision.
- [ ] **`once()` is copied into four test files, and two copies never give up.**
`session-extras.test.ts` and `readme.test.ts` reject after 5000 ms; `session.test.ts` and
`tls.test.ts` wait forever, so an event that never fires still hangs the run the way an
unclosed listener used to. One shared, guarded copy closes the rest of that class.
- [ ] **A peer whose message ids share one base logs a refused merge on every send.** `smsc01-000123`
and `smsc01-000124` carry the same base, so `DlrMerger` merges the first message and refuses
every one after it, one log line per send. Left at `info` — nothing the operator can fix is
wrong — but a rate guard or silence may suit it better. Raised by review, 2026-08-30.
- [ ] **`submit_multi` and the broadcast commands** encode and decode, but nothing exercises them
end to end. The interop suite is the natural place.
- [ ] **Move to TypeScript 7** once `typescript-eslint` supports it; `renovate.json` pins TypeScript
below 6.1 for exactly that reason.
- [ ] **Coverage reporting.** `node --test --experimental-test-coverage` works today; nothing
publishes the numbers.
- [ ] **An `onReceipt` hook.** Receipt text is only loosely specified and operators disagree on it,
but `dlrFromPdu()` is wired into `IncomingRequests` with no seam of its own: an application
facing a format we do not parse has to take the whole PDU on `onRequest` and reimplement the
dispatch, which owns the response as well.
Mirror the `onRequest` seam — return a `Dlr` to own the receipt, `undefined` to fall through
to the built-in parser.
## Declined
- **Merge state surviving a process restart.** Declined by AGENTS.md goal 7, maintainer's call,
2026-09-02. A restart loses every incomplete receipt group and a peer has no reason to resend one it
already had answered, so the loss is real — but surviving it means handing the application the merge
state to persist, which the scope floor covers as squarely as holding the state here would, and
which publishes the shape of `DlrMerger`'s groups against goal 6. Nothing is foreclosed: the seam
can still be added after 1.0.0 as a minor.
- **Throughput throttling — a TPS cap, and backing off on `ESME_RTHROTTLED`.** Declined by AGENTS.md
goal 7: an operator's rate limit is scoped to the account, while the widest thing this library owns
is a session, so a bucket here cannot see a second process binding the same account and is wrong in
exactly the case it exists for. `sendSms()` surfaces `ESME_RTHROTTLED` to the caller instead, and
`maxOutstanding` stays — a window slot frees on the peer's next response, which is self-limiting in
a way a rate ceiling is not.