T3/disconnect final send

CLAUDE.md

@@ -40,13 +40,13 @@ The **server** compiles under **C++98** (`Makefile`). The **test suite** compile
 
 The event loop and all command handling live in a single **`Server`** instance (`src/Server.cpp`, `include/Server.hpp`):
 
-- `Server::run()` — the **single** `epoll_wait` call (annotated in `src/Server.cpp`; epoll lifecycle/ctl ops live behind `libcpp98::Reactor`). Client fds register for `EPOLLIN | EPOLLOUT`. `EPOLLIN` → `handleClientInput`, `EPOLLOUT` → `handleClientOutput`. The loop also runs `checkTimeouts()` (PING/PONG keepalive, SENDQ sweep) and fires `onTick` on extensions every pass.
+- `Server::run()` — the **single** `epoll_wait` call (annotated in `src/Server.cpp`; epoll lifecycle/ctl ops live behind `libcpp98::Reactor`). Client fds register for `EPOLLIN` only; `EPOLLOUT` is armed on demand — a per-tick `_epollMask` reconcile sweep in `run()` sets `EPOLLIN | EPOLLOUT` while a client has queued output and drops back to `EPOLLIN` once `_out` drains (an always-armed `EPOLLOUT` is level-triggered and would busy-loop). `EPOLLIN` → `handleClientInput`, `EPOLLOUT` → `handleClientOutput`. The loop also runs `checkTimeouts()` (PING/PONG keepalive, SENDQ sweep) and fires `onTick` on extensions every pass.
 - `_clients` (`map<int fd, Client*>`) and `_channels` (keyed by **casemapped** name — display case lives in `Channel::_name`) are the live state. `Server` owns and frees these pointers, plus all registered extensions (deleted in reverse order).
 - `main.cpp` ignores `SIGPIPE`, traps `SIGINT`/`SIGTERM` into `Server::isRunning`, and calls `registerExtensions(server)` (tier-dependent) before `run()`.
 
 **Extension seam** (`include/ext/IServerExtension.hpp`): everything optional plugs in through this observer interface — hooks for lifecycle (`onServerStart`, `onTick`), client events (`onClientRegistered`, `onClientDisconnect`), channel events (`onJoin`, `onPart`), interception (`onCommand` — fired only where ERR_UNKNOWNCOMMAND would go, so extensions can add commands like `FILE` but never shadow RFC ones; `onPrivmsg` — fired per non-channel target so virtual participants claim messages; `reservesNick`), foreign fds (`ownsFd`/`onFdEvent` + public `Server::registerExternalFd` — how PlatformBus multiplexes its socket into the same epoll), and `onAudit` fan-out (`Server::audit()` → AuditLog extension). The kernel never names a concrete extension.
 
-**I/O is fully buffered, never blocking.** `Client` delegates to `libcpp98::BufferedSocket` (512-byte line cap, 64 KiB SENDQ — overflow latches and the client is disconnected at the next sweep point, never mid-broadcast). Every extracted line passes one sanitizer stripping stray `\r`/`\0` (kills IRC line injection; `\x01` CTCP/DCC bytes pass untouched). Handlers never call `send()` directly — they queue via `Server::sendToClient`/`sendReply`/`Client::queueMessage`, drained on `EPOLLOUT` (plus a best-effort flush in `disconnectClient`). When iterating extracted messages, code re-checks `_clients.find(fd)` after each because a handler may have disconnected the client.
+**I/O is fully buffered, never blocking.** `Client` delegates to `libcpp98::BufferedSocket` (512-byte line cap, 64 KiB SENDQ — overflow latches and the client is disconnected at the next sweep point, never mid-broadcast). Every extracted line passes one sanitizer stripping stray `\r`/`\0` (kills IRC line injection; `\x01` CTCP/DCC bytes pass untouched). Handlers never call `send()` directly — they queue via `Server::sendToClient`/`sendReply`/`Client::queueMessage`, drained on `EPOLLOUT`; `disconnectClient` deliberately does **not** flush before closing (see Known traps). When iterating extracted messages, code re-checks `_clients.find(fd)` after each because a handler may have disconnected the client.
 
 **Command dispatch** (`Server::dispatchCommand`) is a linear `if (cmd == ...)` chain, split across files by category:
 - `CommandRegistration.cpp` — CAP, PASS, NICK, USER, `completeRegistration` (timing-safe password check via `libcpp::str::eq_consttime`)
@@ -69,7 +69,7 @@ Dispatch enforces a **registration gate**: only CAP/PASS/NICK/USER/QUIT/PONG run
 
 ## Testing
 
-Tests use Google Test but also feed every result into **PostMan** (`vendor/PostMan.cpp`), a styled Unicode-table reporter — `tests/test_main.cpp` bridges the two via a custom `TestEventListener`. `tests/Makefile` builds all of `src/` *except* `main.cpp` (linking `tier_full.cpp` as the one `registerExtensions` definition). Protocol-level suites share `tests/TestHarness.hpp` (TCP `TestClient` + `IrcServerTest` fixture; subclass and override `portBase()` per suite, `onServerReady()` to inject probe extensions). Test files: `test_message`, `test_client`, `test_channel`, `test_bot`, `test_integration`, `test_robustness`, `test_security`, `test_filetransfer`, `test_extensions`, `test_libcpp98`. Suite is 138/138; PostMan's leak counter is atomic and `assertNoLeaks` takes `const char*` (a `std::string` argument would count itself as a leak — keep it that way).
+Tests use Google Test but also feed every result into **PostMan** (`vendor/PostMan.cpp`), a styled Unicode-table reporter — `tests/test_main.cpp` bridges the two via a custom `TestEventListener`. `tests/Makefile` builds all of `src/` *except* `main.cpp` (linking `tier_full.cpp` as the one `registerExtensions` definition). Protocol-level suites share `tests/TestHarness.hpp` (TCP `TestClient` + `IrcServerTest` fixture; subclass and override `portBase()` per suite, `onServerReady()` to inject probe extensions). Test files: `test_message`, `test_client`, `test_channel`, `test_bot`, `test_integration`, `test_robustness`, `test_security`, `test_filetransfer`, `test_extensions`, `test_libcpp98`. Suite is 139/139; PostMan's leak counter is atomic and `assertNoLeaks` takes `const char*` (a `std::string` argument would count itself as a leak — keep it that way).
 
 ## Known traps
 
@@ -81,4 +81,6 @@ Tests use Google Test but also feed every result into **PostMan** (`vendor/PostM
   reaps an RST is **timing-dependent** when `EPOLLIN` and `EPOLLHUP` arrive in
   the same event — do NOT assume `EPOLLERR|HUP` alone covered RST before that
   errno removal (`RobustnessTest.AbruptDisconnectViaRST` guards it now).
+- **No flush-on-disconnect**: `disconnectClient()` closes the fd without a
+  final best-effort `send()` of whatever is still queued in `_out` — two unpolled best-effort send()s were removed (T3): the flush in disconnectClient() and the MAX_CLIENTS "Server full" rejection in acceptClient(). The kernel now has no send() outside the EPOLLOUT-gated path in handleClientOutput(). Known accepted regressions: ERR_PASSWDMISMATCH (464) / the 001-005 burst on immediate post-registration QUIT, and the "Server full" ERROR when at MAX_CLIENTS, no longer reach the client (socket closes with zero bytes). Compliant recovery = deferred teardown (T4).
 

src/Server.cpp

@@ -235,11 +235,12 @@ void Server::acceptClient()
 	// Connection cap: reject gracefully instead of exhausting fds
 	if (_clients.size() >= MAX_CLIENTS)
 	{
-		const char rejection[] = "ERROR :Server full\r\n";
-		send(clientFd, rejection, sizeof(rejection) - 1, 0); // best effort
-		close(clientFd);
-		Log::warn("connection rejected: MAX_CLIENTS reached");
-		return;
+	    /* Reject by closing only — no unpolled send() here either (T3, same rule
+	    ** as the removed disconnect flush). A courtesy "Server full" line would
+	    ** need the deferred-teardown machinery tracked as T4. */
+	    close(clientFd);
+	    Log::warn("connection rejected: MAX_CLIENTS reached");
+	    return;
 	}
 
 	// Set non-blocking
@@ -486,15 +487,6 @@ void Server::disconnectClient(int fd, const std::string &reason)
 	for (size_t i = 0; i < _extensions.size(); ++i)
 		_extensions[i]->onClientDisconnect(*this, *client, reason);
 
-	// Best-effort flush of replies queued before this disconnect (e.g. the
-	// 001-005 burst when the client QUITs immediately after registering).
-	// Single non-blocking send; whatever the kernel refuses is dropped.
-	if (client->hasPendingData())
-	{
-		const std::string &pending = client->getSendBuffer();
-		send(fd, pending.c_str(), pending.size(), 0);
-	}
-
 	_epollMask.erase(fd);
 	removeFromEpoll(fd);
 	close(fd);

tests/COVERAGE.md

@@ -2,7 +2,7 @@
 
 Cross-references every item on the 42 evaluation sheet against what the repo
 actually proves today. Built from a scan of `tests/`, `scripts/audit.sh`, and
-`src/` on `main` (`4e78d69`).
+`src/`, current as of the T1–T3 kernel-compliance work.
 
 **Legend:** ✅ covered · 🟡 partial · 🔴 gap / risk · 🧭 manual-only (not automatable)
 
@@ -17,8 +17,8 @@ FancyLogSink, shrinking the surface an evaluator can question.
 |---|---|---|---|
 | Makefile; compiles `-Wall -Wextra -Werror -std=c++98`; exec `ircserv` | ✅ | `audit.sh` §compile | Also `make mandatory/bonus/full` all build |
 | **Only one** poll/epoll/select | ✅ | `audit.sh` §single-poll; verified 1 `epoll_wait` (Server.cpp:158) | PlatformBus multiplexes into the same epoll — no 2nd wait |
-| poll called before each accept/recv/send | 🟡 | architectural (all I/O is epoll-event-driven) | **Exception:** best-effort `send()` in `disconnectClient` (Server.cpp:503) is not preceded by a write event → decide defend vs remove |
-| **errno not used to trigger action after recv/send/accept** | 🔴 | — (unaudited, untested) | **TOP RISK.** `errno==EAGAIN` checks after recv (289), send (326), accept (234). No retry-loop, but errno branches control flow. Refactor recommended (EPOLLERR/HUP already handles real errors) |
+| poll called before each accept/recv/send | ✅ | architectural (all I/O is epoll-event-driven) | **Resolved (T3):** both unpolled best-effort sends removed (`disconnectClient` flush + `acceptClient` "Server full") — zero exceptions remain in the kernel |
+| **errno not used to trigger action after recv/send/accept** | ✅ | `RobustnessTest.AbruptDisconnectViaRST` | **Resolved (T2):** errno branching removed from recv/send/accept; real errors reaped by the `EPOLLERR\|EPOLLHUP` branch in `run()`. Also cured a latent `EINTR` false-positive disconnect |
 | fcntl only `F_SETFL, O_NONBLOCK` | ✅ | `audit.sh` §fcntl; verified (Server.cpp:104,250) | PlatformBus fcntl also compliant, but it's an extra |
 
 ---
@@ -87,11 +87,16 @@ FancyLogSink, shrinking the surface an evaluator can question.
 ## Prioritized action list (defense order)
 
 **P0 — eliminatory, do first (a gap here = 0, everything else is moot):**
-1. **errno-after-I/O** (A). Audit → refactor recv/send/accept to not branch on
-   `errno` (rely on `EPOLLERR|EPOLLHUP`), or prepare an airtight justification.
-   Guard: `Robustness.AbruptDisconnect` must stay green.
-2. **Un-polled `send()` in `disconnectClient`** (A/B). Decide: remove (accept
-   losing app-buffered QUIT/ERROR text) or defend as a teardown flush.
+1. ✅ **errno-after-I/O** (A) — resolved (T2): errno branching removed from
+   recv/send/accept; real errors handled by the `EPOLLERR|EPOLLHUP` branch.
+   Guard: `RobustnessTest.AbruptDisconnectViaRST`.
+2. ✅ **Un-polled `send()` in `disconnectClient`** — resolved via Option A
+   (T3): the flush was removed, closing the last literal exception to
+   "poll before every send" in the kernel. Accepted regression: 464 /
+   welcome-burst no longer reach a client disconnected in the same tick
+   they were queued (see `CLAUDE.md` "Known traps"). Deferred-teardown
+   recovery (Option C) is tracked as future work (T4), out of this plan's
+   scope.
 
 **P1 — scored / high-value robustness:**
 3. **Non-operator denial tests** (E) — the operator score is 0–5 and the sheet

tests/test_integration.cpp

@@ -45,11 +45,18 @@ TEST_F(IntegrationTest, WrongPassword)
 	tc.sendCmd("USER wrongpw 0 * :Test");
 	std::this_thread::sleep_for(std::chrono::milliseconds(300));
 
-	std::string reply = tc.recvAll();
-	/* Server queues 464 but may disconnect before flushing.
-	   Either we see 464 or the connection was simply closed. */
-	EXPECT_TRUE(reply.empty() || tc.hasNumeric(reply, "464"))
-		<< "Expected either ERR_PASSWDMISMATCH or closed connection";
+	/* T3 (Option A): disconnectClient() no longer drains _out before
+	   close(fd). sendReply(464) and disconnectClient() are called
+	   synchronously in the same path (completeRegistration) with no
+	   epoll_wait in between, so the 464 structurally never arrives.
+	   Deterministic contract: zero bytes delivered and the connection
+	   closed (EOF) -- recv() checked for n == 0 rather than recvAll(),
+	   which can't distinguish EOF from a timeout on a still-open socket. */
+	char buf[64];
+	ssize_t n = recv(tc.fd(), buf, sizeof(buf), 0);
+	EXPECT_EQ(n, 0)
+		<< "Expected connection closed with zero bytes delivered "
+		   "(464 is queued but never flushed before close)";
 }
 
 TEST_F(IntegrationTest, NoPassword)
@@ -61,9 +68,13 @@ TEST_F(IntegrationTest, NoPassword)
 	tc.sendCmd("USER nopw 0 * :Test");
 	std::this_thread::sleep_for(std::chrono::milliseconds(300));
 
-	std::string reply = tc.recvAll();
-	EXPECT_TRUE(reply.empty() || tc.hasNumeric(reply, "464"))
-		<< "Expected either ERR_PASSWDMISMATCH or closed connection";
+	/* Same path as WrongPassword (!hasPassSent() hits the same branch in
+	   completeRegistration) -- same deterministic contract. */
+	char buf[64];
+	ssize_t n = recv(tc.fd(), buf, sizeof(buf), 0);
+	EXPECT_EQ(n, 0)
+		<< "Expected connection closed with zero bytes delivered "
+		   "(464 is queued but never flushed before close)";
 }
 
 TEST_F(IntegrationTest, UnregisteredCommand)