diff --git a/AGENTS.md b/AGENTS.md index a197a6a..ff4389b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -87,8 +87,9 @@ src/ ``` 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 -`createSms()`, imported as a type only. +uses `pdu`, and `client`/`server` use `session`. The ways back up are the `Session` handed to +`createSms()`, 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/src/session.ts b/src/session.ts index 4b337f7..614c0b5 100644 --- a/src/session.ts +++ b/src/session.ts @@ -72,8 +72,8 @@ export class Session extends EventEmitter { private readonly timers: LinkTimers; private readonly transport: PduTransport; - private closed = false; - private ended = false; + /** `ended` is final: end() stops the reconnect loop before any attach() can run. */ + private lifecycle: 'attached' | 'ended' | 'torn-down' = 'attached'; /** A listener that throws is the application's bug; it must not become ours. Hard rule 1. */ override emit( @@ -186,7 +186,7 @@ export class Session extends EventEmitter { const sent = built.err ? { err: built.err } : this.transport.write(built.buffer); // A peer that unbinds and drops the link takes our response with it; that is not a failure. - if (sent.err && !this.closed) { + if (sent.err && this.lifecycle === 'attached') { this.log.warn('session - could not answer a request', { cmdName, message: sent.err.message, @@ -221,12 +221,12 @@ export class Session extends EventEmitter { */ async unbind(): Promise { const drained = await this.drain(undefined); - const wasOpen = !this.closed; + const wasOpen = this.lifecycle === 'attached'; // now(), not send(): a drain refuses a send, and the unbind goes out either way. const sent = wasOpen ? await this.outgoing.now({ cmdName: 'unbind' }) : { err: new Error('Session is closed') }; - const closedOnUnbind = wasOpen && this.closed; + const closedOnUnbind = wasOpen && this.lifecycle !== 'attached'; this.end(); @@ -325,7 +325,7 @@ export class Session extends EventEmitter { private attach(sock: Socket): void { this.transport.attach(sock); - this.closed = false; + this.lifecycle = 'attached'; } /** Stops new sends and waits out the messages we hold and the requests already issued. */ @@ -371,17 +371,17 @@ export class Session extends EventEmitter { } private emitClose(): void { - if (this.ended) return; + if (this.lifecycle === 'ended') return; - this.ended = true; + this.lifecycle = 'ended'; this.outgoing.linkLost(false); this.emit('close'); } private teardown(): void { - if (this.closed) return; + if (this.lifecycle !== 'attached') return; - this.closed = true; + this.lifecycle = 'torn-down'; // Read once: clear() reports lost segments, and a listener could stop the loop between reads. const retrying = this.retrying(); @@ -445,15 +445,15 @@ export class Session extends EventEmitter { } private resetTimers(): void { - if (this.closed) return; + if (this.lifecycle !== 'attached') return; this.timers.reset(); } private onClose(): void { - if (this.reconnectLoop && !this.reconnectLoop.isStopped()) { + if (this.retrying()) { this.teardown(); - this.reconnectLoop.schedule(); + this.reconnectLoop?.schedule(); return; } diff --git a/todo.md b/todo.md index f602931..f3dced1 100644 --- a/todo.md +++ b/todo.md @@ -199,9 +199,24 @@ next work ([decision](docs/decisions.md#internals-and-tests)). ### Locality — next, ahead of everything below; 5–6 today, and the gate is 7 -- [ ] **Lift the held-message and shutdown code to Locality 7, and confirm it with a scoring run.** - The 2026-09-27 run capped every seat there; a run reading 7.0 or above also retires the #30 - decision. +A second four-seat run on 2026-09-28, after #35–#40, read 6, 6, 7 and 6 again, Locality 5, 5, 6 +and 5. Every seat ranked the session's lifecycle hardest and least wanted to modify it. + +- [ ] **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. +- [ ] **Let `Session` own its bind state.** `client.ts` and `server.ts` write `boundAs`, `loggedIn` + and `peerInterfaceVersion` onto the session from outside, and `loggedIn` duplicates + `boundAs !== undefined`. A method such as `session.bound(bindType, declaredVersion)` with the + three fields read-only changes the public surface a hand-wired SMSC uses, so it needs the + maintainer's call first. All four seats. +- [ ] **Name what `request()`, `carry()` and `now()` each skip.** Three ways onto the wire differ + only in which of the drain, the gate and the window they bypass, and none of the names says + which. Three seats. +- [ ] **Give the held-message flow one place a reader can follow it.** Whether a drain still waits + on a message is spread over `emitSms()`, `MessageHold`, the session's rejection route and + `Sms.isHeld()`. Four seats. +- [ ] **Shrink the `IncomingDeps` closure bag.** 16 lambdas, six of them repeated in + `SmsHandlers`, which makes every inbound call path indirect. Two seats. ### Correctness @@ -244,6 +259,23 @@ next work ([decision](docs/decisions.md#internals-and-tests)). README.md or in MIGRATION.md — write it in the same change as the rule, so it is worded once. From the stability and product-owner reviews of #18. +- [ ] **Build every error from a thrown value through `errorFrom()`.** `reconnect-loop.ts` (in + `run()`'s catch and in `bringUp()`) and `client.ts`'s connect still call `String(thrown)`, + which throws on a null-prototype object; in `run()` that lands as an unhandled rejection from + a `void`ed promise, against hard rule 1. From the 2026-09-28 scoring run. + +- [ ] **Leave `sms.smsId` alone when `sendResp()` fails.** `sms.ts` assigns `options.smsId` before + the lost-link check and before the write, so a failed answer still renames the message and a + later `sendDlr()` names an id the peer was never given. From the 2026-09-28 scoring run. + +- [ ] **Refuse a timeout past 2³¹−1 ms, as `connectTimeout` already is.** `checkLimits()` bounds + `responseTimeout`, `idleTimeout`, `shutdownTimeout` and `reassemblyTimeout` from below only, + and Node fires a larger delay after 1 ms. From the 2026-09-28 scoring run. + +- [ ] **Keep a bare ESC out of GSM detection.** `gsmRegex` in `defs/encodings.ts` admits `\x1B`, + so `"\x1B("` is detected as GSM, goes out as 0x1B 0x28 and arrives as `{`. From the + 2026-09-28 scoring run. + ### Throughput — goal 6, and the default window is where we are slowest - [ ] **Close the gap to jsmpp at `maxOutstanding: 10`.** Measured 2026-09-20 against the same sink,