Skip to content

fix(plugin-hono-server): keep a graceful close from cutting off a response still flushing to a slow client - #31255

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-05yvmq
Nov 7, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-05yvmq

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #31208

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:服务优雅关闭(HonoHttpServer.close())时,一个已经调用 end()、但还在往慢客户端发送的响应,不再在关闭开始那一刻被掐断;空闲的 keep-alive 连接改为在没有响应仍在发送时才统一释放。
  • 为什么改:每次 os serve / 运行时关停或重新部署时,正在下载大响应的客户端会收到被截断的内容(实测 32 MiB 只收到 3,919,702 字节),原因是 Node 自己的 server.close() 把「已 end 未发完」的连接当作空闲连接销毁了。
  • 风险与回滚:只改 @objectstack/plugin-hono-server 一个文件,patch 级;关闭窗口的强制关闭兜底(默认 10 秒)原样保留,所以最坏情况是空闲连接晚一点释放,不会让关停卡住;回滚即还原本 PR。
  • 先看:packages/plugins/plugin-hono-server/src/adapter.ts 的 closeIdleConnectionsOnceFlushed(什么时候推迟清扫),再看 tryListen 里把它装到 server 实例上的那三行,以及 trackForDrain 里响应 'close' 时补跑被推迟的清扫。
  • 两分钟验证:pnpm --filter @objectstack/plugin-hono-server exec vitest run src/adapter-drain.test.ts,预期 9 个用例全绿。
  • 需要你做的:无,席位落地。

What changes

packages/plugins/plugin-hono-server/src/adapter.ts, the drain start of HonoHttpServer.close():

  • What destroyed the response, measured. On Node 22.23.4 (.nvmrc is 22, engines is >=22.0.0, and every CI job reads .nvmrc or pins '22'), a Socket.prototype.destroy probe during the reordered slow-reader case recorded two destroys of the flushing socket, in this order: first Server.closeIdleConnections called from httpServerPreClose from Server.close (adapter.ts server.close(...)), then the adapter's own server.closeIdleConnections() call on the next line. So Node's own server.close() sweep destroys it first; the adapter's call would have done the same. Both skip a connection only while socket._httpMessage.finished is false, and end() sets finished before the bytes are flushed.
  • The guard. tryListen puts a closeIdleConnections on the server instance that wraps Node's: closeIdleConnectionsOnceFlushed runs Node's sweep only when no tracked response is writableEnded && !writableFinished. Otherwise it holds the sweep back. Node's httpServerPreClose calls server.closeIdleConnections() on the instance (the probe's stack shows it), so both sweeps at drain start go through the guard, and server.close() still stops accepting and clears Node's connection-check interval at once.
  • Each flushing connection closes after its own 'finish'. That path is PR fix(plugin-hono-server): end a graceful close with its last in-flight request; cut or budget seven slow Test Core cases #31189's, unchanged: the response's 'close' destroySoon()s its own socket once it has finished and nothing else is owed on that connection.
  • The held-back sweep then runs. Each tracked response's 'close' (in trackForDrain) re-asks the guard when a sweep is held back, so idle keep-alive connections are released as soon as no response is flushing, not a keep-alive window later. Node's raw server-wide sweep is still never run from a response's 'close' (the PR fix(plugin-hono-server): end a graceful close with its last in-flight request; cut or budget seven slow Test Core cases #31189 M1 hazard): the only sweep there is the guarded one, which runs only when nothing is flushing.
  • The backstop is untouched. The closeAllConnections() timer and its window are byte-identical. A reader that never drains is now held until the window ends instead of being destroyed at drain start.

Why this route and not the other one the card names. Stopping the accept without the sweep means calling net.Server.prototype.close instead of server.close(). That skips the other half of httpServerPreClose, clearInterval(server[kConnectionsCheckingInterval]), an interval only reachable through a private symbol, so it would keep the closed server alive in a timer; and calling server.close() later to clear it emits 'close' a second time when the connections have already drained. Deferring the whole server.close() keeps accepting new connections during the flush. The instance guard keeps Node's close() whole and changes only when its sweep runs, on every Node the CI runs (22) and any with the sweep (19 and later).

Tests

packages/plugins/plugin-hono-server/src/adapter-drain.test.ts, three new cases:

  • The reordered slow-reader pin. A 32 MiB response calls end() to a paused raw socket before close() starts; the case asserts the premise (writableEnded true, writableFinished false), then the client receives every byte against Content-Length. An idle keep-alive connection sits beside it and must be released within 1 s of the flush ending (the keep-alive timer would hold it 5 s). Red on main (df22adc3, adapter unchanged since 714dda8f): every byte of the body arrived: expected 3919702 to be 33554432.
  • Control: an idle keep-alive connection is released at drain start. Nothing is flushing, so the sweep runs at once: close() and the connection end within 1 s.
  • Control: the backstop still force-closes a reader that never drains. A paused reader of a 32 MiB response with a 300 ms window: close() takes at least 250 ms (held for the window, not swept at drain start) and under 2 s, and the reader gets fewer than 32 MiB.
Ablations

Through scripts/ablation-replace.mjs on the committed head, anchor hit once, restore proven by blob == HEAD and an empty git diff HEAD. The suite imports ./adapter from src/, so no build sits on the path.

  • A1 (head ab2845c8), the guard's flushing check disabled (if (false && res.writableEnded && !res.writableFinished)): 2 failed / 7 passed. a response that ended before close() but is still flushing to a slow reader arrives whole: expected 3919702 to be 33554432; the drain window still force-closes a reader that never drains: expected 1 to be greater than or equal to 250.
  • A2 (head 1502afc5), the held-back sweep never re-run (the if (sweep) this.closeIdleConnectionsOnceFlushed(sweep); line deleted): 1 failed / 8 passed, the slow-reader case Test timed out in 5000ms (the idle keep-alive connection holds close() open).

Acceptance notes

  • The guard relies on Node calling the instance method. httpServerPreClose reads server.closeIdleConnections() dynamically in Node 22 (measured by the probe's stack). If a future Node called the prototype directly, the slow-reader pin goes red again; it does not fail silently.
  • Idle connections wait while something flushes. While an ended response is still flushing, the sweep is held back, so an idle keep-alive connection stays open until the flush ends (then it is released at once) or the window ends. A request arriving on it meanwhile is answered Connection: close by PR fix(plugin-hono-server): end a graceful close with its last in-flight request; cut or budget seven slow Test Core cases #31189's request-time line. Under a stream of new slow flushes the backstop decides, as before.
  • A pipelined response queued behind another counts as flushing once it has ended (its bytes sit in outputData), which only holds the sweep back until it is written. A dead connection's responses drop out with the connection's entry, so they cannot hold the sweep.

Verification

Gate union derived by node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack on the final head ab2845c8 (origin/main merged once, at 733efbd4): 63 families, each exit code captured before any pipe.

  • 63 run, exit 0 each. --ran: "63 derived famil(ies) accounted for — 63 run, 0 NOT-MEASURED". Three first read exit 3 (prerequisite, not measured) and were re-run green: check-plugin-teardown-shape --self-test after git fetch --depth=1 origin 621a4876 for its fixture, and check:dual-build-cjs-loads and check:lean-entry-closure after a full pnpm turbo run build --concurrency=2 under the lock (VERDICT command-exit 0). check:dts-closure and check:published-files were read again on the full build, exit 0.
  • pnpm --filter @objectstack/plugin-hono-server build && typecheck && test under the lock on ab2845c8: VERDICT command-exit 0, Test Files 38 passed (38), Tests 484 passed (484). The package tsconfig.json includes src/**/*, so typecheck compiles the test file.
  • pnpm lint scoped: eslint --no-inline-config over the two touched source files, exit 0.
  • No dogfood battery: no ! in the title and no (narrowing) arm.
  • CI families are read from this PR's check-runs, not re-run here.

Generated by Claude Code

…onse still flushing

Node's server.close() sweeps idle connections first thing and counts a
response as done once it has called end(), before its bytes are flushed,
so a large response still going to a slow reader at drain start was
destroyed mid-body. The sweep (Node's and the adapter's own call) now
waits until no ended response is flushing; each such connection closes
after its own finish, and idle connections are released right after.
The drain window's force-close is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5puKTX6kQoJs8iKrFr
…e instance guard

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5puKTX6kQoJs8iKrFr
@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
  • 1 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 169ff7e53889d275c7db1b1bd0e22e37791c58df → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 714f5e0f6d7e577dd70b00857bcc240ee2765fe1 — the merge of head ab2845c82053c7e6d35286e2c7cd7713145b5dee into base 169ff7e53889d275c7db1b1bd0e22e37791c58df, 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 714f5e0f6d7e577dd70b00857bcc240ee2765fe1 && git checkout 714f5e0f6d7e577dd70b00857bcc240ee2765fe1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 169ff7e53889d275c7db1b1bd0e22e37791c58df ab2845c82053c7e6d35286e2c7cd7713145b5dee && git checkout -B drift-repro 169ff7e53889d275c7db1b1bd0e22e37791c58df && git merge --no-ff ab2845c82053c7e6d35286e2c7cd7713145b5dee

node scripts/docs-audit/affected-docs.mjs --json 169ff7e53889d275c7db1b1bd0e22e37791c58df

⚠️ 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 169ff7e53889d275c7db1b1bd0e22e37791c58df → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-project-manager · session_01FEkQ5o8PE7ZtVjcor51wBZ

维护者速读

  • 改了什么:服务优雅关闭(HonoHttpServer.close())时,一个已经调用 end()、但还在往慢客户端发送的响应,不再在关闭开始那一刻被掐断;空闲的 keep-alive 连接改为在没有响应仍在发送时才统一释放。
  • 为什么改:每次 os serve / 运行时关停或重新部署时,正在下载大响应的客户端会收到被截断的内容(实测 32 MiB 只收到 3,919,702 字节),原因是 Node 自己的 server.close() 把「已 end 未发完」的连接当作空闲连接销毁了。
  • 风险与回滚:只改 @objectstack/plugin-hono-server 一个文件,patch 级;关闭窗口的强制关闭兜底(默认 10 秒)原样保留,所以最坏情况是空闲连接晚一点释放,不会让关停卡住;回滚即还原本 PR。
  • 先看:packages/plugins/plugin-hono-server/src/adapter.ts 的 closeIdleConnectionsOnceFlushed(什么时候推迟清扫),再看 tryListen 里把它装到 server 实例上的那三行,以及 trackForDrain 里响应 'close' 时补跑被推迟的清扫。
  • 两分钟验证:pnpm --filter @objectstack/plugin-hono-server exec vitest run src/adapter-drain.test.ts,预期 9 个用例全绿。
  • 需要你做的:无,席位落地。

席位意见:接受。优雅关闭时,已结束但还在往慢客户端发送的响应不再被 Node 的空闲连接清理截断;清理改为等所有在发送的响应发完再跑,排空窗口的强制关闭兜底不变。

席位核对 — ACCEPT · domain:server#5 · Head-sha ab2845c82053c7e6d35286e2c7cd7713145b5dee · 2026-11-07T20:32Z

  • Shape: 3 files, +151/−11, read from the diff. adapter.ts: the server instance's closeIdleConnections is wrapped (so Node's own server.close() → httpServerPreClose reaches the wrapper too) by closeIdleConnectionsOnceFlushed, which holds the sweep while any tracked response is writableEnded && !writableFinished and re-runs it from trackForDrain's 'close' handler; deferredIdleSweep is cleared on close. The force-close backstop is unchanged.
  • Premise measured: on Node 22 Node's server.close() sweep destroys the flushing socket first, the adapter's own call second (dev reading); the reordered slow-reader pin is red on main df22adc3 (expected 3919702 to be 33554432).
  • Pins: reordered slow-reader case gets every byte (ablated red, restored); CONTROL idle keep-alive still closes at drain start; CONTROL backstop still force-closes a reader that never drains.
  • Gates: head check-runs 35 success, 13 skipped, none red or pending. Test Core (8/8) was red once on this head: two examples/app-todo loop-body-iteration.test.ts cases timed out at 5000 ms inside bootTodoKernel (no HTTP server in that test, nothing this diff reaches); re-run once by the seat, green. check-expected-skips: OK, all 13 in main's roster (exit 0). No governed path. The dev's dispatch-gates --commands: 63 derived, 63 run, all exit 0. Mergeable state clean.
  • Accepted deviations: none beyond the order.
  • Out of scope: none. Clause-②: no, Metadata-protocol: none, Skills-docs: n/a. Changeset @objectstack/plugin-hono-server patch.
  • Landing: ungoverned. Fixes #31208. Ready + auto-merge.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 7, 2026 20:33
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 7, 2026 20:33
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit fd3ab0a Nov 7, 2026
72 of 74 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-05yvmq branch November 7, 2026 21:07
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.

plugin-hono-server: graceful close() destroys a response that has ended but is still flushing to a slow client (truncated body)

1 participant