fix(#5): clear waitFor timer and catch poll rejections

[Jackallink]

Closes: Jackallink/qm-integration#5

src/runs/postgres-run-store.ts waitFor polls getRun every 250ms with setInterval and rejects after a setTimeout. Two leaks: when finish() resolves on the first terminal status, clearInterval was called but clearTimeout was not, so the timeout still fired later and rejected an already-settled promise; and the poll itself ran as a fire-and-forget .then() with no .catch, so a DB blip became an unhandled rejection.

Call clearTimeout in finish() alongside clearInterval so the timeout can never race the resolved promise. Attach .catch(() => undefined) to the poll branch to swallow transient errors and let the next tick recover.

Affected tests

• test/postgres-store.test.ts — PG-gated, skipped locally (test/remote-turn-*-pg.test.ts pattern)
• Affected tests covered by CI under test:pg shard

Reviewer checklist

• Fresh-context pass (AGENTS.md)
• Screenshot: N/A
• Live dev instance: required (PG-only paths)
• Cross-package: no plugin changes

[Jackallink]

Independent review — 2026-09-04

Findings on PR #4

1. Timer reject path leaks events listener (separate bug, same file)

The diff correctly adds clearTimeout(timer) to the finish() success path (good — fixes the race where timer fires after resolve). It also correctly adds .catch() to the poll branch (good — prevents unhandled rejection on DB blip).

But the timer reject branch at line 308-314 (existing code, untouched by this PR) does:

const timer = setTimeout(() => {
  if (done) return;
  done = true;
  clearInterval(poll);
  // ❌ missing events.off(runId, onSettle)
  reject(new Error(`run ${runId} did not finish within ${timeoutMs}ms`));
}, timeoutMs);

If the timer fires first (timeout), onSettle remains attached to events. Future events.emit(runId, run) from the same emitter will still call onSettle. finish() short-circuits via done = true, so no functional damage in the short term, but every timeout'd waitFor accumulates one orphan listener.

Note that finish() correctly does events.off(runId, onSettle), so the success path doesn't have this issue. Only the timer-reject path does.

Out of scope for #4 (which was scoped to the poll branch and the finish/timer race). Recommend filing a follow-up issue; the fix is one line.

2. events.once(runId, onSettle) + events.off(runId, onSettle) reference equality

Verified: onSettle is a function declaration hoisted inside the new Promise constructor; both once and off use the same closure-captured reference, so off correctly removes the listener. No issue here.

3. PG-gated test scope

PR body says "test/postgres-store.test.ts — PG-gated, skipped locally". Acceptable given #4 is a PG-only path. Suggest adding a unit-level assertion that doesn't require PG: e.g., a mock getRun that always rejects, observe that .catch() swallows the rejection without unhandled event. Otherwise the .catch(() => undefined) fix is unverified locally.

4. AGENTS.md check

• ✅ Refs: Jackallink/qm-integration#5
• ✅ No upstream issue numbers
• ✅ 0 new comments

Item 1 is the only structural finding. Item 3 is a test-coverage suggestion. Both are non-blocking if acknowledged as follow-ups.

[Jackallink]

Correction to my earlier review finding — 2026-09-04

My previous review comment claimed F4.1: "timer reject path misses events.off(runId, onSettle)". That is wrong — retracted.

Verified against both fork/main and the PR branch: the timer reject path already has events.off(runId, onSettle) in the base code:

const timer = setTimeout(() => {
  if (done) return;
  done = true;
  clearInterval(poll);
  events.off(runId, onSettle);   // present in base
  reject(new Error(...));
}, timeoutMs);

The PR does not touch the timer path. No listener leak there. Apologies for the noise — the diff's addition of clearTimeout(timer) in finish() only closes the timer-vs-poll race on the success path, which is correct and sufficient.

Remaining valid finding on this PR: F4.2 (PG-gated test does not cover the .catch path — suggest a mock-based unit test).

[Jackallink]

Deep-review pass addendum — 2026-09-04

Verified on branch fix/postgres-waitfor-poll-leak-5 (5e05ffd):

1. F4.1 retracted (see my earlier correction comment) — the timer reject path already has events.off(runId, onSettle) in base. My mistake.

2. F4.2 stands: PG-gated suite cannot run locally (no PG instance in this worktree env). Typecheck passes on the branch (tsc --noEmit, exit 0). The .catch(() => undefined) on the poll branch is unverified by unit tests — suggest a mock-based test: getRun rejects, assert the promise settles via timeout rather than crashing with an unhandled rejection.

3. The clearTimeout(timer) addition in finish() is correct: it closes the race where the timer fires after the poll already resolved — previously that late timer would call reject() on a settled promise (harmless no-op in JS, but the error object construction and the done guard made it confusing). No issue found in the change itself.