@qwen-code /triage

updated the PR description to match the template. code is unchanged.

@qwen-code /triage

Thanks for the PR, @tt-a1i!

Template looks good ✓

On direction: this is a clear-cut bug fix — the close-handler setTimeout at L636 was the only reconnect timer in QQChannel that wasn't being tracked or unref'd, which could keep the daemon alive up to 30s during backoff. Issue #5413 identified it precisely, and this PR addresses it directly. Aligned with the project's stability goals.

On approach: the scope is minimal and correct — 4 lines of production code that match the existing store+unref pattern already used at L845-846. No unrelated changes. The test additions are justified: the MockWebSocket infrastructure (~40 lines) is necessary to exercise the real close handler path rather than mocking internals. Feels right-sized.

Moving on to code review. 🔍

— Qwen Code · qwen3.7-max

Code Review

Independent proposal: to fix "close-handler setTimeout not tracked/unref'd," I would (1) store the setTimeout() result in the existing this.reconnectTimer field, (2) call .unref() on it, (3) confirm disconnect() already clears it, and (4) add a test that triggers the real close handler. This is exactly what the PR does.

The production change is 4 lines in QQChannel.ts — stores the timer and unrefs it at the close handler, matching the identical pattern already at L845-846. No correctness bugs, no security concerns, no regressions. The field declaration (L129) and disconnect() cleanup (L314-316) already exist, so the fix slots in cleanly.

The test adds a MockWebSocket class to exercise the real dialGateway → close handler → reconnect timer path. It verifies: timer is stored, timer is unref'd (hasRef() === false), disconnect() calls clearTimeout and nulls the field. Solid regression coverage — this test would fail without the fix.

Test Results

$ cd packages/channels/qqbot && npx vitest run src/send.test.ts

 ✓ src/send.test.ts (40 tests) 16ms

 Test Files  1 passed (1)
      Tests  40 passed (40)
   Duration  418ms

$ npx tsc --noEmit --project packages/channels/qqbot/tsconfig.json
(clean — no errors)

$ npx prettier --check packages/channels/qqbot/src/QQChannel.ts packages/channels/qqbot/src/send.test.ts
All matched files use Prettier code style!

Real-Scenario Testing

N/A — this fixes internal reconnection timer logic in the QQ bot channel. Triggering it requires a live QQ bot gateway connection, which is not available in CI. The unit test exercises the actual close handler path end-to-end and is the appropriate verification for this change.

— Qwen Code · qwen3.7-max

This is a clean, well-scoped bug fix. The problem is real (unref'd timer keeping daemon alive during backoff), the fix is the minimum viable change (4 production lines matching an existing pattern), and the test covers the actual code path rather than mocking around it. My independent proposal was identical to what the PR does — that's a good sign.

The test infrastructure additions (~40 lines for MockWebSocket) might look like a lot relative to the 4-line fix, but they're testing the real close-handler → reconnect-timer path, which is strictly better than mocking setTimeout and asserting it was called. This is the kind of test that catches real regressions.

No concerns. Ships the fix, adds coverage, gets out of the way.

Approving. ✅

— Qwen Code · qwen3.7-max

✅ Maintainer verification — real local + tmux testing

Built the PR head (05f4deb4, Node v22.22.2) in an isolated worktree and reproduced #5413 with a real process-liveness A/B. TL;DR: a tiny, correct, CI-green timer-leak fix — the close-handler reconnect timer is now tracked + .unref()'d and disconnect() clears it; mutation-confirmed, and the live A/B shows the pending reconnect timer used to keep the process alive (~2s) and now lets it exit (+7ms). Recommend merge.

What it fixes

Issue #5413: the QQ bot close handler scheduled its reconnect backoff with a bare setTimeout(() => this.reconnectWithRetry(), delay) — untracked and not .unref()'d. So (a) disconnect() had no reference to clear, and (b) the pending reconnect delay was enough to keep the Node/daemon process alive on exit. The fix (QQChannel.ts:636-640):

this.reconnectTimer = setTimeout(() => this.reconnectWithRetry(), delay);
this.reconnectTimer.unref();

disconnect() already clears this.reconnectTimer (clearTimeout + null, lines 314-317), and the other reconnect path (reconnectWithRetry, line 849) was already tracked + unref'd — so this just makes the close-handler path consistent with it.

Real process-liveness A/B (tmux) — reproduces #5413

Scheduled a reconnect timer the way the close handler does (attempt-1 delay = min(1000·2¹, 30000) = 2000ms), then let the process try to exit:

The un-unref'd timer keeps the process alive for the full reconnect delay — and in production that delay grows to 30s and every fire schedules another, so the daemon never exits. .unref() lets it exit immediately. (This A/B demonstrates the setTimeout + unref mechanism the channel now applies; the regression test below proves the channel actually applies it — hasRef() === false on the real close-handler-scheduled timer.)

Regression test + mutation

The PR's test (tracks and unrefs reconnect timers scheduled by close handler) drives the real close handler (dialGateway → mock ws → emit('close', 4001)), then asserts reconnectTimer !== null, timer.hasRef() === false (unref'd), and that disconnect() clearTimeouts it and nulls it.

• FIXED → ✅ pass.
• Mutation (revert the hunk back to a bare setTimeout) → ❌ FAIL (expected null not to be null): the untracked timer leaves reconnectTimer null, so disconnect() can't clear it and it isn't unref'd → the test genuinely guards both the tracking and the unref.

CI & the bot's review

• CI is green on the current head (05f4deb4): Lint ✅ · Test (macos / ubuntu / windows · Node 22) ✅ · CodeQL ✅.
• The qwen-code-ci-bot CHANGES_REQUESTED is template-only — it explicitly says "the fix itself looks spot-on … exactly the right approach" and only asked to reformat the PR body. The current body already uses ## What this PR does / ## Why it's needed / ## Reviewer Test Plan, so that request is satisfied and the CR is stale — it can be dismissed.

Evidence

Verdict

Correct, minimal, CI-green fix for a real process-liveness leak (#5413): tracking + .unref() + disconnect()-clear are all verified, the regression test is non-vacuous (mutation-proven), and the tmux A/B reproduces the symptom and its fix. Recommend merge. (Backs my approval; the CI bot's template-only CHANGES_REQUESTED is satisfied by the current body and can be dismissed.)

this.reconnectTimer = setTimeout(() => this.reconnectWithRetry(), delay);
this.reconnectTimer.unref();

_{Method: isolated worktree build of 05f4deb4 (exit 0) · regression test (tracks and unrefs … close handler: hasRef()===false + disconnect clears) · source-mutation (revert the close-handler hunk → test fails expected null not to be null) · real process-liveness A/B (ref hangs ~2000ms / unref exits +7ms = #5413) · CI check-runs green (Lint + Test ×3 + CodeQL).}

✅ Local verification — built & real-tested

I checked this out on fix/qqbot-close-reconnect-timer, built @qwen-code/channel-qqbot from source, and verified it with the full static suite plus a real end-to-end test (real ws, real timers, the actual compiled code — no mocks). Verdict: correct, minimal, and safe to merge.

Environment: Node v22.22.2, Linux · built with tsc --build.

Static checks

Real integration test (the part that matters)

The unit test mocks ws. To independently confirm the claim from #5413 — a pending reconnect must not keep the daemon alive — I drove the real private dialGateway() of the compiled QQChannel against a real local WebSocketServer that accepts the socket and immediately closes it with code 4001 (the exact path that schedules a reconnect). Ran twice: once with the fix reverted (BEFORE), once applied (AFTER).

Test A — timer is tracked, unref'd, and cleared on disconnect()

Test B — does the pending reconnect keep the Node process alive?
(reconnect backoff preset to 16 s; a 2.5 s unref'd watchdog decides the verdict)

So with the fix a daemon that's mid-backoff exits cleanly; without it the referenced reconnect timer pins the event loop open — precisely the #5413 symptom.

============ TEST A — tracking + unref + disconnect clears (real ws, real timers) ============
---- BEFORE (fix reverted) ----
{"label":"BEFORE","marker":"SCHEDULED","attempts":1,"expectedDelayMs":2000,"realTimerScheduled":true,"trackedInField":false,"fieldIsCaptured":false,"hasRef":true}
{"label":"BEFORE","marker":"AFTER_DISCONNECT","clearTimeoutCalledWithTimer":false,"reconnectTimerNull":true,"timerStillRefdAfterDisconnect":true}
---- AFTER  (PR applied)  ----
{"label":"AFTER","marker":"SCHEDULED","attempts":1,"expectedDelayMs":2000,"realTimerScheduled":true,"trackedInField":true,"fieldIsCaptured":true,"hasRef":false}
{"label":"AFTER","marker":"AFTER_DISCONNECT","clearTimeoutCalledWithTimer":true,"reconnectTimerNull":true,"timerStillRefdAfterDisconnect":false}

============ TEST B — does a pending reconnect keep the daemon process alive? ============
---- BEFORE (fix reverted): expect STILL_ALIVE, exitcode 2 ----
{"label":"BEFORE","marker":"SCHEDULED","attempts":4,"expectedDelayMs":16000,"realTimerScheduled":true,"trackedInField":false,"fieldIsCaptured":false,"hasRef":true}
{"label":"BEFORE","marker":"LOOP_RELEASED","activeResources":["TCPServerWrap","PipeWrap","Timeout"]}
{"label":"BEFORE","marker":"VERDICT","result":"STILL_ALIVE","meaning":"a referenced reconnect timer kept the daemon process alive","activeResources":["PipeWrap","Timeout"]}
==> exitcode=2 wall=2.63s
---- AFTER  (PR applied): expect PROCESS_EXITS_CLEANLY, exitcode 0 ----
{"label":"AFTER","marker":"SCHEDULED","attempts":4,"expectedDelayMs":16000,"realTimerScheduled":true,"trackedInField":true,"fieldIsCaptured":true,"hasRef":false}
{"label":"AFTER","marker":"LOOP_RELEASED","activeResources":["TCPServerWrap","PipeWrap"]}
{"label":"AFTER","marker":"VERDICT","result":"PROCESS_EXITS_CLEANLY","meaning":"reconnect timer did NOT keep the daemon process alive"}
==> exitcode=0 wall=0.13s

Code notes

• The fix makes the close-handler path consistent with the existing reconnectWithRetry() fallback, which already did this.reconnectTimer = setTimeout(...); .unref().
• disconnect() already cleared this.reconnectTimer — the close-handler timer just wasn't being stored, so this supplies the missing half. No new state, minimal surface, no change to reconnect timing.
• Live QQ gateway not exercised (out of scope, as noted) — but the real-ws harness covers the exact changed branch deterministically.

LGTM 👍

_{Verified locally with Claude Code (Opus 4.8).}