Skip to content

fix(plugin-hono-server): end a graceful close with its last in-flight request; cut or budget seven slow Test Core cases - #31189

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-30920-test-core-slow-cases
Nov 7, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-30920-test-core-slow-cases

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Requested by 田騏 · project thread

Fixes #30920

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:Hono 服务器优雅关闭时,正在处理的请求一结束就关掉它自己的连接(未发响应头的回 Connection: close,已发 keep-alive 头的在全部字节发完后只关自己这条连接,不碰别的连接);关闭期间的连接记账按连接存放、连接断开即清掉;另外七条慢用例里两条砍掉重复工作、四条按「CI 最慢读数 ×3」给了单独超时,一条经测量不需要改。
  • 为什么改:两条 drain 用例在空闲机器上也要 3–4 秒,实测原因是关闭时还在跑的请求结束后变成空闲 keep-alive 连接,服务器要等客户端自己断开(fetch 3 秒、node 代理 4 秒);线上服务关闭同样会白等这段时间。其余七条在 CI 上已用到 5 秒默认超时的一半以上,负载一高就会把 main 和合并队列变红。
  • 风险与回滚:生产代码只改 plugin-hono-server 的关闭路径;一条响应结束时只释放它自己的连接,别的连接上还在发送的大响应不受影响(有慢读者 32 MiB 用例钉住),流水线请求在连接断开后不再被留在内存里;drain 超时强关的兜底不变;其余都是测试文件。回滚就是 revert 本 PR。
  • 先看:packages/plugins/plugin-hono-server/src/adapter.ts 的 trackForDrain、responsesOwedOn 与 close();adapter-drain.test.ts 新增的慢读者、流水线断连与迟到请求头三条用例;packages/client/src/auth-rotated-session-token.test.ts 模块顶层新增的 fileServer() 预建。
  • 两分钟验证:pnpm --filter @objectstack/plugin-hono-server exec vitest run src/adapter-drain.test.ts src/adapter-tls-listener.test.ts --reporter=verbose,应看到 12 条全过,两条 drain 用例约 0.2 秒(改前 3.2 秒与 4.2 秒)。
  • 需要你做的:无,席位落地。

What changes

The drain (packages/plugins/plugin-hono-server/src/adapter.ts, patch changeset). close() calls closeIdleConnections() once, when the drain starts. A connection whose request is still running at that moment is skipped, and Node never runs the release again. When the response ends, the connection becomes an idle keep-alive socket, and server.close() waits until the client drops it. The client drops it after the server's Keep-Alive: timeout=5 hint minus its own margin: 3 s for fetch (undici) and 4 s for the node:http(s) agent. Measured with a socket trace (see Tests): the response finishes at about 220 ms, the server socket waits with a 6000 ms timer, and close() resolves only when the client sends FIN, at 3232 ms (fetch) and 4216 ms (agent).

During a drain the adapter now does two things:

  • a response whose headers are not written yet goes out with Connection: close, so Node ends the socket once it is sent and the client knows not to reuse it. This covers responses in flight when close() starts, and requests whose headers finish arriving on an open connection after the drain began;
  • a response whose keep-alive headers were already sent when the drain began (a stream, for example) ends its OWN connection with destroySoon() once it has finished (every byte handed to the socket) and no other response on that connection is pending. It never calls the server-wide closeIdleConnections(): Node counts a response as done once it has called end(), before its bytes are flushed, so a sweep at that moment would destroy any other connection whose response is ended but still flushing to a slow reader.

The drain bookkeeping is kept per connection (responsesOwedOn) and dropped when the connection closes. A response pipelined behind another never gets a socket if the connection dies, so it never emits 'close'; keying by connection keeps it from being held until close(). The drain window still force-closes whatever outlasts it.

The seven other cases. Each budget follows the rule PR #30919 used: 3x the slowest Test Core reading on the card, rounded up to the next second, with the readings and the arithmetic in a comment at the case. No global timeout, no skip, no retry.

Case Action Local before (ms) Local after (ms)
metadata-protocol two derivations of the same schemas agree exactly cut: one derivation compared with the module-load singleton, not two plus the singleton 743 / 844 / 775 353 / 372 / 410
client login → enable → verifyTotp → disable → deleteUser cut: the file's shared server and its better-auth instance are built at module scope, out of the first case's clock 729 / 800 / 743 566 / 583 / 558
driver-sql sql-driver.ts and the modules split out of it contain none of them budget 8000 (3 x 2499 = 7497; the reading is 1 ms under the rule's over-half-the-default line, and the comment says why it takes the budget anyway) 469 / 442 433 / 313 / 432
plugin-security posture single/group/isolated hidden-column cases budget 9000 (3 x 2949 = 8847) 624-694 489-669
plugin-sharing a refused upgrade write does not block the read budget 8000 (3 x 2605 = 7815) 545 / 458 / 689 499 / 398 / 474
cli the census reaches the live tree budget 8000 (3 x 2629 = 7887) 1291 / 1243 / 1102 1117 / 1234 / 1217
cli the boot this sweep reads from really did compose the deployment no change, see Acceptance notes 10222 / 11341 / 10996 n/a

Tests

adapter-drain.test.ts pins each part:

  • the in-flight case asserts the response carried Connection: close;
  • a streamed response whose keep-alive headers went out before close() completes, and close() resolves less than 1000 ms after its body ends (the defect waits about 3000 ms);
  • a raw connection writes half a request's headers, close() starts, the rest arrives: the answer is Connection: close;
  • a slow reader's 32 MiB response calls end() during the drain and is still flushing (asserted) when another keep-alive stream ends: every byte arrives;
  • four pipelined requests on one raw connection, the connection destroyed: the drain bookkeeping holds none of them afterwards.

Ablation, one part at a time, through scripts/ablation-replace.mjs on head 586338ffbd (the tests import ./adapter from source, so no build is involved; each restore verified as blob == HEAD with an empty git diff HEAD):

  • the round-1 server-wide closeIdleConnections() in place of the own-socket release: the slow-reader case goes red, expected 3919730 to be 33554432 bytes;
  • without dropping a connection's entry on its 'close': the pipelined case goes red, expected 3 to be +0;
  • without the request-time Connection: close line: the late-headers case goes red, expected 'keep-alive' to be 'close'.

The whole new test file against the round-1 adapter.ts (2ff6bb0735): the slow-reader case fails (expected 3919730 to be 33554432) and the pipelined case fails (the round-1 set holds responses, so its retained count reads NaN against the expected 4); the late-headers case passes, as the round-1 code had that line.

Socket trace of close() with one request in flight (before / after)

Before (fetch):

[   56 ms] close() called
[  219 ms] server: response finished; shouldKeepAlive=true _last=false
[  224 ms] client: status 200, connection=keep-alive, keep-alive=timeout=5
[  226 ms] server socket after finish: timeout=6000
[ 3232 ms] server socket: "end" (peer, the client, sent FIN)
[ 3232 ms] close() resolved

Before (node:http agent, keepAlive): FIN from the client and close() resolved at 4216 ms. With agent: false (no keep-alive) close() resolved at 215 ms, which isolates the keep-alive wait as the cause.

After (fetch):

[   59 ms] close() called
[  227 ms] server: response finished; shouldKeepAlive=false _last=true
[  228 ms] close() resolved
[  237 ms] client: status 200, connection=close
Drain cases, local, before / after
Case Before After
adapter-tls-listener.test.ts drains on close exactly as the plain listener does 4209 ms 214 ms
adapter-drain.test.ts lets an in-flight request complete instead of aborting it 3233 ms 268 ms

Acceptance notes

  • cli unmanaged-tables "the boot this sweep reads from really did compose the deployment" is not changed. Timing trace (instrumented locally, not committed): the case body takes 1-2 ms, its afterEach 3 ms, and its beforeEach 10038-10378 ms, nearly all of it the first bootSchemaStack of the worker. vitest reports hooks inside a case's duration, but the 5 s testTimeout clocks only the body; this beforeEach declares its own 180_000. So the 5 s default cannot time this case out, and a per-case timeout on a 2 ms body would change nothing. The later cases in the file boot in 90-145 ms. The local readings are on a box shared with other builds; the card's CI reading is 3295 ms.
  • client keeps no budget, by the file's own rule. Its header says a recurrence is answered by cutting the case's own work, ⛔ not by a timeout, and its pin ④ refuses a numeric per-case timeout. The cut above moves arrangement out of the clock; what is left is the probe itself (a sign-up and five password or TOTP checks).
  • Found, not fixed here: a response that has called end() but is still flushing at the moment close() STARTS is destroyed mid-body. Node's own server.close() sweeps idle connections (Node 19+), and the adapter's call at drain start does the same; both treat an ended response as done before its bytes are flushed. Reproduced on this branch with the slow-reader case reordered so the big response ends before close(): expected 3919702 to be 33554432 bytes. That path is unchanged from main; reported to the seat for a card.
  • No other file changes. The drain fix is a published-package change, so it ships a patch changeset; the test-only changes in the other six packages publish nothing.

Verification

Head 586338ffbd (round 2, on the round-1 merge of origin/main at 8ef4d30905; the delta against main is these nine files).

  • Typecheck, each touched package: plugin-hono-server, client, metadata-protocol, plugin-security, plugin-sharing, driver-sql, cli: all exit 0.
  • Tests: plugin-hono-server 481/481 on round 2 (478/478 on round 1), client 712/712, metadata-protocol 22525 passed / 26 skipped, plugin-security 5820 passed / 45 skipped, plugin-sharing 1445/1445, driver-sql 3867 passed / 218 skipped, cli --project unit 5603/5603. The cli integration layer is left to CI.
  • Gates derived by node scripts/pm/dispatch-gates.mjs --commands on 586338ffbd: 69 families, 69 run, all exit 0; --ran reconciliation: 0 NOT-MEASURED, 0 UNRUN. check:tenant-audit-census first flagged the new set's .delete() receiver; the set is now a named type in the adapter, and the census passes. check:dts-closure and check:dual-build-cjs-loads first read a dist/ that was being rebuilt and went red on missing files; rerun on the finished build, both exit 0.

🤖 Generated with Claude Code

…ponse ends during a drain

close() released idle keep-alive sockets only at the start of the drain,
so a connection whose request was still running became an idle keep-alive
socket afterwards and server.close() waited for the client to drop it
(3 s for fetch, 4 s for the node:http(s) agent). During a drain a response
whose headers are not yet written now goes out with Connection: close,
and one that already sent keep-alive headers releases its socket through
closeIdleConnections() once it ends.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2ebbZkr2QAEG1x7UC8
- metadata-protocol: the purity case derives once and compares it with the
  module-load singleton instead of deriving twice.
- client: the shared auth server and its better-auth instance are built at
  module scope, out of the first case's clock.
- driver-sql, plugin-security, plugin-sharing, cli census: an explicit
  per-case budget of 3x the slowest Test Core reading, rounded up to the
  next second, with the readings and the arithmetic beside it.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2ebbZkr2QAEG1x7UC8
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 7, 2026
@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-hono-server, touching 6 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/index.mdx (via HonoHttpServer (symbol, a top-level class))
  • content/docs/api/wire-format.mdx (via HonoHttpServer (symbol, a top-level class))
  • content/docs/protocol/kernel/error-handling.mdx (via HonoHttpServer (symbol, a top-level class))
What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 63 of 226 client-bound route-ledger rows — the other 163 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 163: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 59 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 104 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 658d84f5756e616d6418cb0ecf38630e04b4e341 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from edf77727516c530e813ee26b289d218d6243ba83 — the merge of head 586338ffbdd25e1fa9c86965dcfa3534e8993ad0 into base 658d84f5756e616d6418cb0ecf38630e04b4e341, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin edf77727516c530e813ee26b289d218d6243ba83 && git checkout edf77727516c530e813ee26b289d218d6243ba83
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 658d84f5756e616d6418cb0ecf38630e04b4e341 586338ffbdd25e1fa9c86965dcfa3534e8993ad0 && git checkout -B drift-repro 658d84f5756e616d6418cb0ecf38630e04b4e341 && git merge --no-ff 586338ffbdd25e1fa9c86965dcfa3534e8993ad0

node scripts/docs-audit/affected-docs.mjs --json 658d84f5756e616d6418cb0ecf38630e04b4e341

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 658d84f5756e616d6418cb0ecf38630e04b4e341 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

objectstack-fleet Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor Author

domain:devx#2 · marchtian · session_017ihL2ebbZkr2QAEG1x7UC8

Contract review

Served-tier: TIER_DEFAULT
Head-sha: 586338ffbdd25e1fa9c86965dcfa3534e8993ad0
Local-runs: none

① Derived judgments

Sections ① to ③ are the round-1 review of the earlier head 2ff6bb0735, kept as the record of what round 2 had to answer. Its verdict on that head was FAIL (M1, M2 must-fix; S1, S2 should-fix). The Round 2 delta after ③ judges the head named above, and its verdict is the one this record gives.

Read against objectstack-ai/objectstack at head 2ff6bb0735 (net diff over origin/main 620b6e6d40, nine files, +176/−9), card #30920 (body, triage 6684701060, claim 6708147577, dev report 6710121990) and PR #31189's body. Node semantics are read from the Node v22.11.0 sources lib/_http_server.js and lib/_http_outgoing.js (.nvmrc is 22). Served tier: the file list holds no published schema and no governed path, and the changeset's Clause-②: no carries no (narrowing) arm, so the record tool decides the declaration face.

Gates on this head (the check-runs are the verdicts). Green: Lint & Repo Gates and every Lint · lane, Build Core, Governed Surface Queue Guard, Live E2E UI, Dogfood Regression Gate, Check Changeset, Type Check · source gates, Type Check · debt ledger. Still in_progress when this was read: Test Core 1/8–8/8, Type Check · workspace, Type Check · consumer gates, Type Check UI, Test UI 1/16–16/16, Build & E2E UI. Lint UI skipped. So Test Core and TypeScript Type Check have no verdict on this head yet. That is not green.

The drain fix (packages/plugins/plugin-hono-server/src/adapter.ts)

  • Connection: close on responses whose headers are not yet written (HTTP/1.1, TLS): correct. _storeHeader decides the connection header from shouldKeepAlive at the moment _header is built, and headersSent is !!_header. So the close() loop's !res.headersSent check matches that decision point, with no partial state. A response whose header went out close sets _last, and resOnFinish then calls socket.destroySoon() after the flush. That is Node's own close-after-response path. The request-time branch runs in a prependListener('request'), ahead of @hono/node-server's listener, so even a synchronous response cannot write its headers first. close() runs in a later macrotask, so it cannot interleave with a header write. TLS uses the same _http_server path, since https.Server carries the connection tracking and closeIdleConnections(). HTTP/2 is not served: both arms of tryListen build an http/https HTTP/1.1 server, and none is node:http2.
  • ⛔ M1 — the closeIdleConnections() arm can cut another connection's response short. Node 22 skips an idle connection only if (socket._httpMessage && !socket._httpMessage.finished) (_http_server.js 594–608). OutgoingMessage.end() sets finished = true at once (_http_outgoing.js 1146). 'finish', and with it detachSocket, comes only after the last write is flushed (onFinish, resOnFinish 1000–1002). A GET's connection is on the idle list as soon as its request has been parsed. So a connection B whose response has called end() but is still flushing counts as idle and not skippable: for example, a large JSON body handed to end() in one call, going to a slow client. trackForDrain calls the server-wide closeIdleConnections() every time any keep-alive response A ends during the drain, and that call socket.destroy()s B mid-body. Before this PR the same Node call ran only at the single instant the drain began. Now it runs again each time an A ends, for the whole window. The likely A is a stream or SSE response whose headers went out before close(). So the fix trades a keep-alive wait for a truncated response on a concurrent connection. Three claims are false in this case: the JSDoc's "That call skips any socket still writing a response", the changeset's "In-flight requests still complete and are never cut short" (it ships in CHANGELOG.md), and the body's matching sentence. Fix: release only the ending response's own socket. Once res has closed, socket is flushed and detached, so if the drain is on and socket is still open with no _httpMessage assigned, socket.destroySoon() (or end()) it. Never call the server-wide sweep from there. Correct the JSDoc, the changeset and the body. Add a pin: during a drain, a response already end()-ed to a client that is not reading arrives complete (byte count equals its Content-Length) while another keep-alive stream ends.
  • ⛔ M2 — inFlightResponses leaks pipelined responses, on the hot path. The prepended listener adds every response to a strong Set and removes it only on the response's 'close'. A ServerResponse emits 'close' only through resOnFinish, which needs 'finish' and so a socket, or through onServerResponseClose, which fires for socket._httpMessage alone (_http_server.js 257–297, 796–808, 1026–1030). A pipelined request parsed while the one before it is pending gets a response queued in state.outgoing, with res.socket === null. If the socket then dies, that queued response is never assigned and never emits 'close'. It stays in the Set with its req and any body the handler buffered into outputData, until close() clears the set. Any unauthenticated client can pipeline requests and drop the socket, over and over: unbounded memory growth on every request path of a long-running server. Before this PR nothing kept those objects alive past their socket. Fix: tie each entry's life to its socket's life as well as its response's: remove it on the socket's 'close', or track per connection. Pin it: pipeline N requests behind a pending one, destroy the socket, and assert the tracking set returns to empty.
  • Force-close: unchanged. The closeAllConnections() timer block is byte-identical. The new lines only set the flag before it and clear state after it. A late 'close' after the window finds draining === false, or a destroyed socket, and does nothing.
  • Outside a drain: the per-request behaviour is unchanged, because the 'close' handler returns at !this.draining. The per-request work is an extra listener, a Set add/delete and a closure: a small cost I would accept. The Set is not acceptable as it stands, because of M2.
  • Ablation honesty. Both arms the PR names were ablated, with red readings recorded in the body: expected 'keep-alive' to be 'close' and expected 3006 to be less than 1000. Reading the cases, each fails without its half: the in-flight case reaches the close() loop, and the streamed case reaches the closeIdleConnections() arm. S1: the request-time branch if (this.draining) res.shouldKeepAlive = false has no case. A request whose headers complete after close() began goes through it: one mid-headers at drain start, which is active and so not idle, or a pipelined one. Every case stays green with that line deleted. Pin it with a raw socket: write half the request headers, call close(), write the rest, and assert Connection: close.

Test-side changes

  • metadata-protocol derivation: not weaker. The removed flatten(a) vs flatten(b) compared two back-to-back calls in one process state. The kept comparison, the derivation now vs the module-load singleton, spans the wider gap in time and state that the case's comment says it guards. A nondeterministic or state-dependent derivation that the a/b pair would catch also differs from the singleton. The added expect(a).not.toBe(REFERENCE_SITES) closes a hole the old form had: a memoised deriveReferenceSites() returned a === b === singleton and passed vacuously. deriveReferenceSites() builds a fresh index each call (reference-sites.ts:630, singleton at :729).
  • client shared server at file load: no new shared state. The cases already shared one fileServer(). Only who pays for building it moved: from the first case's clock to collection. The warm-up's engine is destroyed by closeEngines() before the new build. The new engine joins engines and is destroyed by afterAll(closeEngines). Every case still signs up its own user through its own client. Pin ④ still holds: one column-0 await signedIn();, one beforeEach, no }, NUMBER);.
  • Timeouts. The budgets follow the rule in share-link-password-webcontainer.test.ts: 3x the slowest Test Core reading, rounded up to the next second, with readings and arithmetic at the case. The numbers check out: 3×2629→8000, 3×2499→8000, 3×2949→9000, 3×2605→8000. None is a global raise, a skip or a retry. No comment line opens with test-budget:, so readTestBudget sees no stray declaration. plugin-security's file-level test-budget: 20 header is untouched. Note: that rule applies to readings over half the default, and driver-sql's 2499 ms is 1 ms under that line. The card asked for a budget anyway, and the budget is harmless, but the comment's "The rule is 3x the slowest reading" drops the threshold.
  • cli unmanaged-tables left unchanged: justified. The case body is synchronous assertions over stack. The boot is in beforeEach under its own 180_000. vitest's testTimeout wraps only the body, while the reported duration includes the hooks. So the 5 s default cannot red this case, and a per-case timeout would bind nothing. The bootSchemaStack cost inside a clocked hook predates this PR. The dev recorded it as an out-of-scope observation.

Seat's choice: client cut only, no timeout. Agree.

The file header says a recurrence is answered "by cutting the case's own work, ⛔ not a timeout". Pin ④ enforces the literal form. A named-constant budget would slip past pin ④'s [\d_]+ regex while breaking the recorded rule, so "no timeout" is the only reading consistent with the file. The cut is real: server and better-auth construction moved out of ①'s clock, 729–800 → 558–583 ms locally. If CI reds it, the lever is the probe's own work, as the header says.

Landing

  • Fixes #30920: present.
  • Body vs tree: the body names the nine files the tree carries. Its table and readings match the code comments. It repeats M1's false "skips any socket still writing a response" claim, which must be corrected with the code.
  • Governed: no. None of the nine paths is a register row. 185 changed lines, under 3,000. The owning seat lands it through the queue after ACCEPT and a fully green head.
  • Changeset: present, '@objectstack/plugin-hono-server': patch, with both declaration lines. The six test-only packages publish nothing. Its prose must drop or narrow "never cut short" (M1).

Must-fix (FAIL):

  • M1: the server-wide closeIdleConnections() in the 'close' arm can destroy a concurrent connection whose response is ended but still flushing, so a response is truncated. Release only the ending response's own socket, correct the JSDoc, changeset and body claims, and add the slow-reader pin.
  • M2: inFlightResponses never drops a queued pipelined response whose socket dies, so memory grows without bound on the request hot path and any client can trigger it. Tie entries to the socket's life and add the pipelined-then-destroyed pin.

Should-fix:

  • S1: pin the request-time shouldKeepAlive = false branch (raw-socket case, request headers completing after close()).
  • S2: in the driver-sql budget comment, state the rule with its over-half threshold, or say why a 2499 ms reading takes a budget anyway.

② Semver level

patch on @objectstack/plugin-hono-server: correct. Clause-②: no, Metadata-protocol: none, and none takes patch. The change alters shutdown timing and connection headers during a drain, and nothing a metadata author writes. No other package publishes a change. The level stands after M1 and M2 are fixed. Only the changeset's prose changes.

③ Boundary flags

  • Published package: plugin-hono-server ships the drain path and the per-request listener. Both must-fixes sit in shipped code.
  • Hot path: the per-request listener is new on every HTTP request (M2).
  • Spec, ADR-0087 and governed surfaces: untouched. Not Tier H or Tier S. Under the size line.
  • Gates: Test Core and TypeScript Type Check were still running on this head. ⛔ Do not arm until both conclude success, on a head carrying M1 and M2.

Round 2 delta — head 586338ffbd

Read: git diff 2ff6bb0735..586338ffbd (four files, +207/−21: adapter.ts, adapter-drain.test.ts, the driver-sql budget comment and the changeset), dev delta report 6710694928, and the PR body as the seat patched it. Node semantics are from the v22.11.0 sources lib/_http_server.js, lib/_http_outgoing.js and lib/net.js. Nothing was built, run or checked out.

M1: fixed. The 'close' arm no longer touches the server. It calls socket.destroySoon() on the ending response's own connection, and only under four conditions:

  • the drain is on;
  • no other response that connection owes is still pending (owed.size === 0);
  • res.writableFinished is true;
  • the socket is neither destroyed nor ended.

The server-wide closeIdleConnections() survives only at drain start, unchanged from main. The checks against Node 22 hold:

  • When the arm runs. A response's 'close' comes from emitCloseNT, queued by resOnFinish. That runs on 'finish', which onFinish emits from the last write's callback, after detachSocket. Or it comes from onServerResponseClose when the socket dies, and the socket.destroyed guard returns in that case.
  • writableFinished is true by then. Its getter (_http_outgoing.js 175–183) is finished && outputSize === 0 && (!socket || socket.writableLength === 0). Once the response is detached, it reads true.
  • Every byte is flushed before the close. Socket.prototype.destroySoon (net.js 788–796) calls end(). That queues FIN behind every pending write and calls destroy only on the socket's own 'finish'. Nothing queued is dropped. TLS sockets inherit the same method.
  • "Nothing else pending" is correct under pipelining. Every parsed request adds its response to its connection's set in the prepended 'request' listener, including responses queued with no socket yet. resOnFinish assigns the next queued response synchronously, before emitCloseNT runs on the next tick. So a pending pipelined response always keeps owed.size above 0, and the connection is left for that response's own arm, or for Node's _last close when its header went out close. A request whose bytes arrive after the release meets a FIN or a reset. That is the ordinary keep-alive close race, the same one closeIdleConnections() has, and no worse.
  • The claims now match the code. The JSDoc carries the ⛔ against a sweep, the changeset reads "closes its own connection once every byte of it has been flushed. Other connections are not touched", and the body says the same. "Never cut short" is gone everywhere.

M2: fixed. The bookkeeping is a Map from each connection to a Set of the responses it owes, keyed by req.socket, which is set even for a queued pipelined response. Each entry is dropped on the socket's own 'close', through one once listener per connection, so there is no per-request socket listener and no MaxListeners risk. An entry can only live while its socket lives, and so does the socket's own memory.

  • No leak from a connection that never closes. A socket that never emits 'close' is an open connection. It already holds that memory itself, and Node's keepAliveTimeout, headersTimeout and requestTimeout bound it.
  • The drain bounds it too. close() force-closes every connection when the window ends, then inFlightResponses.clear().
  • No stale cross-reference. The === created guard keeps a stale listener from deleting a newer entry.
  • The queued responses are collectable. After the entry is dropped, they are referenced only by their own 'close' closures.

The new pins test what they claim.

  • Slow reader (M1). B's 32 MiB response goes to a paused raw socket. It is released after close() starts, so the drain-start sweep cannot reach it. The case asserts the premise outright: writableEnded === true and writableFinished === false. So it cannot pass with B already flushed. Then A's keep-alive stream ends, and the case asserts the byte count against Content-Length. Ablated to the round-1 sweep it is red, and the dev reports expected 3919730 to be 33554432.
  • Pipelined then destroyed (M2). Four requests go in one write. The case asserts 4 tracked, destroys the socket, and asserts 0. The ablation that removes the drop on connection close reads expected 3 to be +0. The helper reads the private field by name. That is accepted coupling for a bookkeeping pin.
  • Late headers (S1). Half the request headers are written, then close(), then the rest. The parser has begun the message, so the connection is active and the drain-start sweep skips it. The response is created after close()'s loop has run, so only the request-time line can flip it, and the ablation reads expected keep-alive to be close. It goes red on the assertion, not by hanging: the round-2 own-connection arm still ends the connection.

S2: done. The driver-sql comment states the over-half threshold, says 2499 ms sits 1 ms under it, and gives the reason it keeps the budget.

The pre-existing hole: outside this PR, unchanged from main. File it as its own card. A response that has called end() but is still flushing at the instant close() starts is destroyed mid-body. On main, close() already calls server.close(), which in Node 22 runs httpServerPreClose and so closeIdleConnections() (_http_server.js 526–529, 573). It then calls closeIdleConnections() again itself. Both use the same finished test that counts an ended-but-flushing response as done. Round 2 adds no call at that instant. Its Connection: close flip touches only responses whose headers are unwritten, and an ended response's headers are always written. So exposure at drain start is identical to main. Round 1 had widened it to every response end in the window; round 2 narrows it back to that single instant. It is a reproducible defect: the dev's reordered slow-reader case reads expected 3919702 to be 33554432 on this branch. Prime Directive #10 owes it a card, for the seat to file. The dev reported it, and the PR body names it under "Found, not fixed here". It does not block this PR.

Nit (no finding). The type OwedResponses alias and the node:net type import sit between import statements, above ./route-pattern. Lint · ESLint is green on this head, so this is style only.

Landing on 586338ffbd.

  • Fixes #30920: present.
  • The body matches the tree.
  • No governed surface.
  • 371 changed lines, under 3,000.
  • patch changeset, unchanged in level.

Check-runs on 586338ffbd, as read. Green:

  • TypeScript Type Check and all four Type Check · lanes
  • Lint & Repo Gates and every Lint · lane
  • Build Core, Build & E2E UI, Governed Surface Queue Guard, Live E2E UI, Dogfood Regression Gate
  • Test Core 5/8 and 8/8
  • 9 of 16 Test UI shards

Still in_progress:

  • Test Core 1–4/8, 6/8, 7/8
  • Type Check UI
  • seven Test UI shards
  • a second Check Changeset run (one run is already success)

Lint UI was skipped. The required Test Core has no verdict yet. ⛔ Do not arm until every required context concludes success on this head.

Round 2: no must-fix, no should-fix remains. The verdict below is on the code at head 586338ffbdd25e1fa9c86965dcfa3534e8993ad0. Landing still waits on that head's required check-runs.

Implemented-by: claude/issue-30920-test-core-slow-cases
Reviewed-by: session_017ihL2ebbZkr2QAEG1x7UC8

VERDICT: PASS

…ction, and track drain state per connection

The round-1 drain called the server-wide closeIdleConnections() whenever a
keep-alive response ended. Node treats a response as done once it has
called end(), before its bytes are flushed, so that sweep destroyed any
other connection whose response was ended but still flushing. The ending
response now destroySoon()s its own socket, once it has finished and no
other response on that connection is pending.

The tracked responses are now keyed by connection and dropped when the
connection closes: a response pipelined behind another never gets a socket
if the connection dies, never emits 'close', and was held until close().

Pins: a slow reader's 32 MiB body arrives whole while another stream ends
during the drain; pipelined responses on a destroyed connection are
forgotten; a request whose headers finish after close() began is answered
Connection: close. The driver-sql budget comment states the rule's
over-half threshold and why a reading 1 ms under it takes a budget.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2ebbZkr2QAEG1x7UC8
… the tenant-audit census can place its receiver

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2ebbZkr2QAEG1x7UC8
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#2 · marchtian · session_017ihL2ebbZkr2QAEG1x7UC8

维护者速读

  • 改了什么:
    • Hono 服务器优雅关闭时,正在处理的请求一结束就只关它自己的连接:还没发响应头的,回 Connection: close;已发 keep-alive 头的,等全部字节发完再关。
    • 关闭期间的连接记账改成按连接存放,连接一断就清掉。
    • 另外 7 条慢测试用例:2 条砍掉重复工作,4 条按“CI 最慢读数 ×3”给了单独超时,1 条经测量不需要改。
  • 为什么改:Test Core 有 9 条用例贴着 vitest 默认的 5 秒超时。其中两条关闭用例要等 3.2 到 4.2 秒,原因是空闲的 keep-alive 连接要等客户端先断开。改完后约 0.2 秒。
  • 风险与回滚:
  • 先看:packages/plugins/plugin-hono-server/src/adapter.ts 的 trackForDrain、responsesOwedOn 和 close(),以及 adapter-drain.test.ts 新增的三条用例。
  • 两分钟验证:运行 pnpm --filter @objectstack/plugin-hono-server exec vitest run src/adapter-drain.test.ts src/adapter-tls-listener.test.ts,应看到 12 条全过,两条关闭用例约 0.2 秒。
  • 需要你做的:无。本 PR 不涉及受管面,席位在 CI 全绿后自行落地。Opus 审查第 2 轮 PASS(6710280454),Head-sha 586338ffbd。

席位意见:可以落地。第一轮的两项必须修都是真问题,第二轮修复后已对照 Node 22 源码核实,三条新用例都做了反向对照(去掉修复即变红)。

席位核对 — ACCEPT · domain:devx#2 · Head-sha 586338ffbdd25e1fa9c86965dcfa3534e8993ad0 · 2026-11-07T17:40Z

  • Shape: 9 files, +362/−9. The seat read the diff, not the report.
    • plugin-hono-server adapter.ts: the drain path only.
      • During a drain, an unwritten-header response goes out with Connection: close.
      • A response that already sent keep-alive headers and ends during the drain destroySoon()s its own connection after 'finish', once nothing else on that connection is pending. There is no server-wide sweep.
      • Tracking is per connection, and an entry is dropped on that connection's 'close'.
    • adapter-drain.test.ts: five pins.
    • Six test files with the slow-case changes.
    • A patch changeset.
  • Review: this is code outside spec, skills and ADR, so it got an Opus review under the 2026-11-07 rule.
    • Round 1 FAILed on 2ff6bb0735:
      • M1: a server-wide closeIdleConnections() per response could destroy another connection's still-flushing response;
      • M2: pipelined responses leaked after their connection died;
      • S1 and S2 were should-fixes.
    • Round 2 fixed all four. The delta review PASSed on this head, checked against the Node 22.11 sources, with no findings open (6710280454).
    • Pins and their ablations: each pin fails without its fix.
      • slow reader, 32 MiB: 3,919,730 of 33,554,432 bytes;
      • pipelined then disconnected: 3 entries left;
      • late headers: keep-alive instead of close.
    • The seat patched the PR body to match round 2. The read-back was identical.
  • Gates:
    • Head checks: 56 success, 11 skipped, none red.
    • check-expected-skips: OK, all 11 skips are in main's roster (exit 0).
    • check-governed-merges: not governed (exit 0). 371 changed lines.
    • The dev's dispatch-gates --commands: 69 derived, 69 run, all exit 0, on this head.
    • plugin-hono-server: 481 of 481 tests pass.
    • git merge-tree against main (66b17a3ed3) is clean.
  • Out of scope:
  • Landing: not governed, Fixes #30920. The seat marks it ready and arms auto-merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test Core: nine cases sit at 2.5-4.2 s on the vitest 5 s default; the two plugin-hono-server drain cases take 3-4 s even on an idle runner

2 participants