271 lines
17 KiB
Markdown
271 lines
17 KiB
Markdown
# todo.md
|
|
|
|
Remaining work for `@larvit/smpp`. Read [AGENTS.md](AGENTS.md) first — the goals and hard rules
|
|
there constrain every item below.
|
|
|
|
This is a working file that 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
|
|
to 26. 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 to 26, Renovate, tag-triggered publish | `.github/workflows/` |
|
|
|
|
Every defect listed in the AGENTS.md table has a regression test naming the behaviour.
|
|
|
|
## Before publishing 1.0.0
|
|
|
|
- [x] `NPM_TOKEN`, which `.github/workflows/release.yaml` needs, is set as an organization secret.
|
|
- [ ] Tag `v1.0.0` to publish. The first publish creates `@larvit/smpp` on npm, provided the token
|
|
can publish under `@larvit`.
|
|
- [ ] `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.
|
|
|
|
## Close 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.
|
|
|
|
**Answer and close as fixed by 1.0.0**, the reply naming what fixed it:
|
|
|
|
- [ ] [#2](https://github.com/larvit/larvitsmpp/issues/2) Tests for the README examples:
|
|
`test/readme.test.ts`.
|
|
- [ ] [#3](https://github.com/larvit/larvitsmpp/issues/3) Tests for flash messages:
|
|
`test/session.test.ts`.
|
|
- [ ] [#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.
|
|
- [ ] [#13](https://github.com/larvit/larvitsmpp/issues/13) Limit a long SMS to fewer segments: the
|
|
`maxSegments` send option.
|
|
- [ ] [#16](https://github.com/larvit/larvitsmpp/issues/16) Support all three bind types: bound and
|
|
enforced in both directions.
|
|
- [ ] [#17](https://github.com/larvit/larvitsmpp/issues/17) `addr_ton`/`addr_npi` should be
|
|
settable: `sendSms()` takes all four, documented and tested.
|
|
- [ ] [#20](https://github.com/larvit/larvitsmpp/issues/20) Tests fail on current dependency
|
|
versions: the mocha suite is gone; `node:test` on Node 18 to 26.
|
|
- [ ] [#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.
|
|
- [ ] [#68](https://github.com/larvit/larvitsmpp/pull/68), a pull request: `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),
|
|
[#67](https://github.com/larvit/larvitsmpp/pull/67),
|
|
[#70](https://github.com/larvit/larvitsmpp/pull/70),
|
|
[#77](https://github.com/larvit/larvitsmpp/pull/77).
|
|
[#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.
|
|
|
|
## Move the repository to Gitea
|
|
|
|
`gitea.larvit.se/larvit/smpp-js` becomes the repository and `github.com/larvit/smpp-js` a push mirror
|
|
of it. Maintainer's call, 2026-09-13. `@larvit/adf-codec` already runs from Gitea, without a mirror.
|
|
|
|
Decide first:
|
|
|
|
- [ ] **Before or after 1.0.0.** Before: 1.0.0's `repository`, `homepage` and `bugs` name the final
|
|
home. After: 1.0.0 ships now from the pipeline that is already green, and the renamed GitHub
|
|
repository redirects the links it carries.
|
|
- [ ] **Provenance.** npm generates it only on GitHub Actions and GitLab CI/CD. Publishing from Gitea
|
|
drops it, as adf-codec's publishing does; publishing from the mirror keeps it, but makes every
|
|
release depend on the mirror being in sync.
|
|
- [ ] **Where issues and pull requests are filed**, Gitea or the GitHub mirror. adf-codec has Gitea
|
|
issues switched off.
|
|
- [ ] **Review bot.** CodeRabbit does not support Gitea.
|
|
|
|
Then:
|
|
|
|
- [ ] Migrate `larvit/larvitsmpp` into `larvit/smpp-js` with Gitea's GitHub migration, as a regular
|
|
repository rather than a pull mirror.
|
|
- [ ] Fast-forward as the only merge style, as on adf-codec, plus what adf-codec lacks: a protection
|
|
rule on `main` requiring a pull request and a green CI status. That is what makes the commit on
|
|
`main` the commit CI tested; GitHub's merge buttons always write a new commit.
|
|
- [ ] Rename the GitHub repository to `larvit/smpp-js` and add it to Gitea as a push mirror.
|
|
- [ ] Move `.github/workflows/` to `.gitea/workflows/` on the `docker-host` runner, with Renovate as a
|
|
scheduled workflow like adf-codec's. Gitea reads `.gitea/workflows/` and falls back to
|
|
`.github/workflows/`; left where they are, the mirror runs every workflow a second time on
|
|
GitHub, the release included.
|
|
- [ ] Point `package.json`, the README badges and links, AGENTS.md and MIGRATION.md at Gitea, and change
|
|
`interop-tests/AGENTS.md` step 3 from squash-merge to fast-forward.
|
|
- [ ] Check how npmjs.com renders the README's relative links (`MIGRATION.md`,
|
|
`interop-tests/README.md`) for a Gitea `repository`; for a GitHub one it resolves them against the
|
|
default branch.
|
|
|
|
## 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.
|