Skip to content

fix: preserve long-running cleanup requests - #291

Open
lindixu6-hash wants to merge 1 commit into
mainfrom
fix/node26-cleanup-lifecycle
Open

fix: preserve long-running cleanup requests#291
lindixu6-hash wants to merge 1 commit into
mainfrom
fix/node26-cleanup-lifecycle

Conversation

@lindixu6-hash

@lindixu6-hash lindixu6-hash commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • stop treating a fully consumed Node request body close event as a client disconnect
  • abort the fetch-style request only on a genuine request abort, premature response close, or socket failure/close
  • derive the Node HTTP idle timeout from the longest profile-cleanup deadline with a 30-second transport grace period
  • cover normal body completion, genuine disconnect deduplication, completed responses, and timeout ordering

Fixes #285
Fixes #286

Validation

  • HOME=/tmp/opencode-mem-lifecycle-test bun test — 453 passed, 1 skipped, 0 failed
  • bun run typecheck — passed
  • bun run build — passed
  • targeted Prettier check — passed
  • git diff --check origin/main...HEAD — passed
  • Node.js v26.8.1 lifecycle probe reproduced the report: a fully consumed POST emits req.close with req.complete=true while res.writableEnded=false; the new guard ignores that event
  • GitHub package-smoke matrix — Ubuntu, Windows, macOS Intel, and Apple Silicon all passed

Boundary

The regression tests exercise the extracted lifecycle policy deterministically. The Node.js 26 probe verifies the runtime event ordering that triggered the original empty response.

@amandeavor amandeavor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Tested locally on Windows (Node v24.16.0 / Bun 1.4.0):

  • \�un test tests/web-server-request-lifecycle.test.ts tests/ai-cleanup.test.ts\ (12/12 passed)
  • \�un run typecheck\ (\ sc --noEmit\ passed cleanly)

The extraction of \�ttachNodeDisconnectHandlers\ cleanly handles the Node 26
eq.close\ event order after full body consumption, and coupling the transport timeout (\NODE_HTTP_IDLE_TIMEOUT_MS) to the longest cleanup deadline plus grace period prevents premature aborts on large profiles.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Align HTTP and AI cleanup timeout hierarchy AI cleanup aborts on premature request close event under Node.js 26

2 participants