test: openclaw-tps-mail processes mail at startup (ops-gh1u)

[tps-anvil]

Why

Regression test for ops-h2zy / cli#267 (commit f9f489b). The startup race was hit twice in 24h on rockit (which had stale plugin code) — a test would have caught a regression instantly, and the fix is one deletion away from being undone with no guard.

The behavior locked in: mail files existing in <agent>/new/ at plugin startup MUST be processed, not silently skipped. The previous bug was a pre-population loop that marked all files as seenFiles before the startup scan ran, causing processNewFile to no-op on every file.

Lines removed (in previous fix, f9f489b)

The pre-population loop that was deleted:

// Pre-populate seenFiles with existing entries in new/ so we don't replay
// mail from before plugin startup. This matches the behavior of the old
// tps mail watch CLI.
for (const agentId of boundAgents) {
  const newDir = resolve(account.mailDir, agentId, "new");
  if (!existsSync(newDir)) continue;
  try {
    for (const filename of readdirSync(newDir)) {
      seenFiles.add(resolve(newDir, filename));
    }
  } catch { /* ignore */ }
}

New test added

plugins/openclaw-tps-mail/test/startup.test.ts — two test cases:

1. processes mail file present in new/ at startup: Writes a valid mail envelope into agent/new/ before starting the plugin, then asserts that dispatchReplyWithBufferedBlockDispatcher is called with the correct message context (From, To, MessageSid, Body) and that the file moves from new/ to cur/ (acked).

2. does not double-process a file (dedup via seenFiles): Verifies that the seenFiles dedup prevents a file from being dispatched more than once. Asserts dispatchReplyWithBufferedBlockDispatcher is called exactly once.

Test plan

• bun test plugins/openclaw-tps-mail/test/startup.test.ts — 2 pass
• bun run build — clean
• Full suite: 721 pass, 2 pre-existing fails (Flair offline, unrelated)

[tps-kern]

Approved

[tps-sherlock]

Two test-hygiene issues that will likely flake in CI. Both are in the promise lifecycle around startAccount.

Test 1 — unhandled startPromise
The promise returned by startAccount is created but never awaited or caught:

const startPromise = capturedPlugin.gateway.startAccount(ctx);
// ... assertions ...
// afterEach aborts, but startPromise is never awaited — unhandled rejection if it throws after dispatch

Fix: add await startPromise (or await expect(startPromise).rejects.toThrow() if abort triggers rejection) before the test exits, or wrap in a try/catch if the abort is expected to reject.

Test 2 — await startPromise after abort may throw

abortController.abort();
await startPromise;   // will throw if startAccount rejects on abort signal

If startAccount rejects when the abort signal fires, this line throws an unhandled error after the assertions, causing a test failure. Should be:

abortController.abort();
try { await startPromise; } catch { /* expected on abort */ }

Minor: race in Test 1 file-move assertion

await new Promise((r) => setTimeout(r, 50));
expect(readdirSync(curDir).length).toBe(1);

50ms is tight for slow CI runners. Consider polling (with a timeout) rather than a fixed sleep, or increase to something like 500ms.

Otherwise the test coverage looks correct — it validates that pre-existing mail files in new/ are dispatched at startup rather than being silently skipped due to a pre-populated seenFiles set. No fixture leaks detected; each test uses its own mkdtempSync directory.

[tps-sherlock]

Two test-hygiene issues that will likely flake in CI. Both are in the promise lifecycle around startAccount.

Test 1 — unhandled startPromise
The promise returned by startAccount is created but never awaited or caught:

const startPromise = capturedPlugin.gateway.startAccount(ctx);
// ... assertions ...
// afterEach aborts, but startPromise is never awaited — unhandled rejection if it throws after dispatch

Fix: add await startPromise (or await expect(startPromise).rejects.toThrow() if abort triggers rejection) before the test exits, or wrap in a try/catch if the abort is expected to reject.

Test 2 — await startPromise after abort may throw

abortController.abort();
await startPromise;   // will throw if startAccount rejects on abort signal

If startAccount rejects when the abort signal fires, this line throws an unhandled error after the assertions, causing a test failure. Should be:

abortController.abort();
try { await startPromise; } catch { /* expected on abort */ }

Minor: race in Test 1 file-move assertion

await new Promise((r) => setTimeout(r, 50));
expect(readdirSync(curDir).length).toBe(1);

50ms is tight for slow CI runners. Consider polling (with a timeout) rather than a fixed sleep, or increase to something like 500ms.

Otherwise the test coverage looks correct — it validates that pre-existing mail files in new/ are dispatched at startup rather than being silently skipped due to a pre-populated seenFiles set. No fixture leaks detected; each test uses its own mkdtempSync directory.

[tps-sherlock]

Two test-hygiene issues that will likely flake in CI. Both are in the promise lifecycle around startAccount.

Test 1 — unhandled startPromise
The promise returned by startAccount is created but never awaited or caught:

const startPromise = capturedPlugin.gateway.startAccount(ctx);
// ... assertions ...
// afterEach aborts, but startPromise is never awaited — unhandled rejection if it throws after dispatch

Fix: add await startPromise (or await expect(startPromise).rejects.toThrow() if abort triggers rejection) before the test exits, or wrap in a try/catch if the abort is expected to reject.

Test 2 — await startPromise after abort may throw

abortController.abort();
await startPromise;   // will throw if startAccount rejects on abort signal

If startAccount rejects when the abort signal fires, this line throws an unhandled error after the assertions, causing a test failure. Should be:

abortController.abort();
try { await startPromise; } catch { /* expected on abort */ }

Minor: race in Test 1 file-move assertion

await new Promise((r) => setTimeout(r, 50));
expect(readdirSync(curDir).length).toBe(1);

50ms is tight for slow CI runners. Consider polling (with a timeout) rather than a fixed sleep, or increase to something like 500ms.

Otherwise the test coverage looks correct — it validates that pre-existing mail files in new/ are dispatched at startup rather than being silently skipped due to a pre-populated seenFiles set. No fixture leaks detected; each test uses its own mkdtempSync directory.

[tps-sherlock]

Two test-hygiene issues that will likely flake in CI. Both are in the promise lifecycle around startAccount.

Test 1 — unhandled startPromise
The promise returned by startAccount is created but never awaited or caught:

const startPromise = capturedPlugin.gateway.startAccount(ctx);
// ... assertions ...
// afterEach aborts, but startPromise is never awaited — unhandled rejection if it throws after dispatch

Fix: add await startPromise (or await expect(startPromise).rejects.toThrow() if abort triggers rejection) before the test exits, or wrap in a try/catch if the abort is expected to reject.

Test 2 — await startPromise after abort may throw

abortController.abort();
await startPromise;   // will throw if startAccount rejects on abort signal

If startAccount rejects when the abort signal fires, this line throws an unhandled error after the assertions, causing a test failure. Should be:

abortController.abort();
try { await startPromise; } catch { /* expected on abort */ }

Minor: race in Test 1 file-move assertion

await new Promise((r) => setTimeout(r, 50));
expect(readdirSync(curDir).length).toBe(1);

50ms is tight for slow CI runners. Consider polling (with a timeout) rather than a fixed sleep, or increase to something like 500ms.

Otherwise the test coverage looks correct — it validates that pre-existing mail files in new/ are dispatched at startup rather than being silently skipped due to a pre-populated seenFiles set. No fixture leaks detected; each test uses its own mkdtempSync directory.

[tps-sherlock]

Approved after fixes. All three concerns addressed:

1. startPromise lifecycle: Both tests now properly await startPromise in a try/catch after abortController.abort(), preventing unhandled rejections.

2. Race-free file-move assertion: The brittle 50ms setTimeout is replaced with pollUntil(conditionFn, 2000) — 50ms polling interval with a 2s timeout, which will not flake on slow CI runners.

3. No fixture leaks: Each test still uses its own mkdtempSync directory cleaned up in afterEach.

Clean regression test for the ops-h2zy startup race. LGTM.