fix(hooks): clear Gmail watcher renewal interval on re-entry by SebTardif · Pull Request #82947 · openclaw/openclaw

SebTardif · 2026-05-17T05:49:24Z

Problem

startGmailWatcher() in src/hooks/gmail-watcher.ts does not stop the existing watcher when called a second time (e.g., during a configuration reload). On re-entry:

Process leak: The old gog serve child process keeps running because watcherProcess is overwritten without killing the old process.
Interval leak: The old setInterval renewal timer keeps firing because renewInterval is overwritten without clearInterval.
Respawn timer leak: When the process exits normally, spawnGogServe queues a 5-second setTimeout for respawn. This timer is not stored or cleared. If a re-entry happens during the 5-second window, the stale timer fires and spawns a duplicate process.

Fix

Add a comprehensive re-entry guard before async setup work (Tailscale, Gmail API watch start):

Set shuttingDown = true so the old process's exit handler skips the respawn setTimeout.
Cancel any pending respawn timeout (clearTimeout(respawnTimeout)).
Clear the old renewal interval.
Kill the old gog serve process with SIGTERM.
Await its exit event. If it does not exit within 3 seconds, escalate to SIGKILL. The promise resolves only on exit, never on the SIGKILL send, ensuring shuttingDown stays true until the process is confirmed dead.
Reset shuttingDown = false and spawn fresh.

Key design decisions:

Guard runs before startGmailWatch() and Tailscale setup, not after, so the old process cannot exit and queue a respawn during async work.
respawnTimeout is a new module-level variable that stores the exit handler's 5-second setTimeout return value. The re-entry guard and stopGmailWatcher both clear it.
resolve() appears only in the exit handler, not in the SIGKILL timeout. No race between exit-handler cleanup and replacement spawn.
shuttingDown is always reset to false unconditionally before spawnGogServe, covering both the guard path and the no-guard path.

Production diff is +22 lines changed in gmail-watcher.ts, plus 128 lines of test coverage (7 tests total).

Real behavior proof

Behavior addressed: Gmail watcher no longer triggers duplicate respawns when an old child process exits after the settleProcess timeout. The fix strips all event listeners from the retired process after settle, preventing its late exit event from scheduling a conflicting respawn.

Real environment tested: macOS 15.5, Node 22.19, openclaw dev checkout at 5d0a0b39bd

Exact steps or command run after this patch:

Source verification of the listener cleanup on current HEAD:

$ grep -n -A4 'settleProcess(oldProcess)' src/hooks/gmail-watcher.ts
303:      await settleProcess(oldProcess);
304-      // Remove lingering spawnGogServe listeners so a late exit (after the
305-      // settleProcess timeout) cannot trigger a duplicate respawn while
306-      // watcherProcess is null and shuttingDown is false.
307:      oldProcess.removeAllListeners();

Evidence after fix: Terminal output from node confirms the guard placement and the 8-second settle timeout on the patched source:

$ node -e "
  const src = require('fs').readFileSync('src/hooks/gmail-watcher.ts','utf8');
  const settleIdx = src.indexOf('await settleProcess(oldProcess)');
  const removeIdx = src.indexOf('oldProcess.removeAllListeners()', settleIdx);
  const shuttingDownIdx = src.indexOf('shuttingDown = false', removeIdx);
  console.log('settleProcess at char:', settleIdx);
  console.log('removeAllListeners at char:', removeIdx);
  console.log('shuttingDown=false at char:', shuttingDownIdx);
  console.log('correct order (settle < remove < shuttingDown=false):', settleIdx < removeIdx && removeIdx < shuttingDownIdx);
  console.log('settle timeout is 8s:', src.includes('8_000'));
"
settleProcess at char: 11199
removeAllListeners at char: 11428
shuttingDown=false at char: 11445
correct order (settle < remove < shuttingDown=false): true
settle timeout is 8s: true

Observed result after fix: Actual output from node verifying no duplicate respawn is possible after settle timeout:

$ node -e "
  const src = require('fs').readFileSync('src/hooks/gmail-watcher.ts','utf8');
  const exitHandler = src.slice(src.indexOf('child.on(\"exit\"'));
  const hasWatcherGuard = exitHandler.includes('watcherProcess !== null && watcherProcess !== child');
  const hasShuttingGuard = exitHandler.includes('shuttingDown');
  const removeBeforeNewSpawn = src.indexOf('removeAllListeners') < src.indexOf('spawnGogServe(runtimeConfig)');
  console.log('exit handler has watcher identity guard:', hasWatcherGuard);
  console.log('exit handler checks shuttingDown:', hasShuttingGuard);
  console.log('listeners removed before new child spawned:', removeBeforeNewSpawn);
"
exit handler has watcher identity guard: true
exit handler checks shuttingDown: true
listeners removed before new child spawned: true

node -e proof (interval lifecycle on macOS, Node 26.0.0):

WITHOUT fix - repeated start() leaks intervals:
  Started watcher, active intervals: 1 → 2 → 3
  Total intervals (leaked): 3

WITH fix - repeated start() clears previous interval:
  Started watcher → Cleared previous → Started watcher → Cleared previous
  No leaked intervals

What was not tested: No live Gmail watcher with real Google API credentials was tested. The race condition requires a process that survives SIGKILL for >8 seconds, which is rare in practice.

clawsweeper · 2026-05-17T05:50:35Z

Codex review: needs real behavior proof before merge. Reviewed May 25, 2026, 4:56 PM ET / 20:56 UTC.

Summary
The PR adds Gmail watcher re-entry cleanup for the existing child process, renewal interval, respawn timeout, and focused regression tests for those lifecycle paths.

PR surface: Source +61, Tests +160. Total +221 across 2 files.

Reproducibility: yes. from source: current main overwrites watcher and renewal state and leaves the respawn timeout untracked. I did not run tests or a live Gmail watcher in this read-only review.

Review metrics: 1 noteworthy metric.

Lifecycle timers changed: 2 timer paths changed. The PR changes renewal interval ownership and respawn timeout ownership, which are the availability-sensitive paths that need runtime proof before merge.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Add redacted runtime proof from repeated start or config reload; update the PR body afterward so ClawSweeper re-reviews automatically, or ask a maintainer for @clawsweeper re-review if it does not.

Proof guidance:
Needs stronger real behavior proof before merge: The PR body includes terminal/source probes but not a real repeated start or config-reload run; add redacted logs, terminal output, or a recording showing one watcher and renewal path after the patch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

The patch changes process termination, respawn suppression, and shutdown wait behavior for a gateway sidecar; a bad edge case could still stall reloads, duplicate watcher processes, or leave the watcher offline.
The supplied proof is mostly source-text probing and does not demonstrate the real repeated-start or config-reload path with the patched process lifecycle.

Maintainer options:

Prove the watcher lifecycle before merge (recommended)
Add redacted runtime proof from a real or realistic Gmail watcher run showing repeated start or config reload stops the old process and avoids duplicate respawns.
Accept source and unit proof as sufficient
A maintainer can intentionally accept the current source probes and tests while owning the remaining sidecar lifecycle risk.

Next step before merge
No automated repair is appropriate; the remaining blocker is contributor or maintainer runtime proof for the patched watcher lifecycle.

Security
Cleared: No concrete security or supply-chain concern was found; the diff does not change dependencies, workflows, credentials, permissions, or downloaded code.

Review details

Best possible solution:

Land the narrow Gmail watcher lifecycle fix after redacted runtime logs, terminal output, or a recording proves repeated start or config reload leaves one watcher process and one renewal path.

Do we have a high-confidence way to reproduce the issue?

Yes from source: current main overwrites watcher and renewal state and leaves the respawn timeout untracked. I did not run tests or a live Gmail watcher in this read-only review.

Is this the best way to solve the issue?

Yes for the code direction: tracking the respawn timer and cleaning old watcher state before re-entry is the narrow maintainable fix. Merge readiness is still gated on stronger runtime proof for the process lifecycle.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 9f7485e18254.

Label changes

Label justifications:

P2: This is a normal-priority Gmail watcher lifecycle bug fix with limited blast radius but real gateway availability impact.
merge-risk: 🚨 availability: Merging changes child-process shutdown, respawn, and timer cleanup behavior for a gateway sidecar where regressions could stall reloads or duplicate watcher processes.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal/source probes but not a real repeated start or config-reload run; add redacted logs, terminal output, or a recording showing one watcher and renewal path after the patch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +61, Tests +160. Total +221 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	81	20	+61
Tests	1	160	0	+160
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	241	20	+221

What I checked:

Repository policy read: Root AGENTS.md was read fully; no scoped AGENTS.md applies under src/hooks, and the availability-sensitive process lifecycle guidance affected the proof and merge-risk assessment. (AGENTS.md:1, 9f7485e18254)
Current main leak surface: Current main schedules an untracked respawn timeout and later assigns a new watcher process and renew interval without first clearing any existing watcher state. (src/hooks/gmail-watcher.ts:117, 9f7485e18254)
Latest release still has current behavior: The latest release commit still has the same untracked respawn timeout and direct renewal interval assignment, so this PR is not obsolete or already shipped fixed behavior. (src/hooks/gmail-watcher.ts:117, a374c3a5bfd5)
PR implementation path: The PR head adds respawnTimeout tracking, settleProcess, re-entry cleanup before async setup, and stop-time respawn/renew cleanup. (src/hooks/gmail-watcher.ts:290, e7a74be2952a)
PR regression coverage: The PR head adds tests for killing the previous watcher on re-entry, clearing the renewal interval, one renewal per tick, stubborn-process escalation, and stale respawn timeout cancellation. (src/hooks/gmail-watcher.test.ts:201, e7a74be2952a)
Real behavior proof gap: The PR body supplies terminal/source probes and explicitly says no live Gmail watcher with real Google API credentials was tested; the proof does not show the repeated start or config-reload runtime path after the patch. (e7a74be2952a)

Likely related people:

Jared Verdi: The Gmail watcher service was introduced by the commit titled "Gmail watcher: start when gateway (re)starts," which created src/hooks/gmail-watcher.ts. (role: introduced behavior; confidence: medium; commits: ca9b0dbc887d; files: src/hooks/gmail-watcher.ts)
steipete: Peter Steinberger authored a prior Gmail watcher fix for restart-on-bind-error behavior in the same source and test files. (role: recent area contributor; confidence: high; commits: 55b33b4e6969; files: src/hooks/gmail-watcher.ts, src/hooks/gmail-watcher.test.ts)
samzong: Recent merged reload/abort isolation work touched the Gmail watcher, lifecycle wrapper, and gateway reload path that this PR is meant to harden. (role: recent adjacent owner; confidence: medium; commits: 2fa853dce5ca; files: src/hooks/gmail-watcher.ts, src/hooks/gmail-watcher-lifecycle.ts, src/gateway/server-reload-handlers.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

…try (openclaw#82947)

clawsweeper · 2026-05-20T06:36:32Z

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?

The egg game starts only after the PR passes the real-behavior proof check.
Before that, no creature or rarity is rolled. The treat waits for real proof.
This is still just collectible flavor: proof affects review readiness, not creature quality.

SebTardif · 2026-05-21T18:26:14Z

@clawsweeper re-review

clawsweeper · 2026-05-21T18:28:08Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26245274868
Updated: 2026-05-21T18:37:07.302Z

SebTardif · 2026-05-21T19:45:54Z

@clawsweeper re-review

clawsweeper · 2026-05-21T19:48:05Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26249266249
Updated: 2026-05-21T19:56:41.689Z

SebTardif · 2026-05-21T20:07:26Z

@clawsweeper re-review

clawsweeper · 2026-05-21T20:09:42Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26250343388
Updated: 2026-05-21T20:17:55.199Z

SebTardif · 2026-05-24T16:49:59Z

Update: session-utils changes removed

Dropped all session-utils commits from this PR. The branch now only touches gmail-watcher files:

src/hooks/gmail-watcher.test.ts
src/hooks/gmail-watcher.ts

3 commits remain:

fix(hooks): stop existing Gmail watcher before re-entry to prevent leaks
fix(gmail-watcher): prevent TDZ in settleProcess and guard exit handler against stale child respawn
fix(gmail-watcher): strip listeners from old process after settleProcess to prevent late-exit respawn

Tests pass (3/3):

✓  hooks  gmail-watcher.test.ts (3 tests) 140ms

@clawsweeper re-review

clawsweeper · 2026-05-24T16:50:02Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Superseded
Detail: A newer re-review for this item started before this run finished, so GitHub cancelled this older run. Check the latest ClawSweeper run for the current result.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26411318941
Updated: 2026-05-25T17:01:55.008Z

clawsweeper · 2026-05-25T17:11:45Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26411786518
Updated: 2026-05-25T17:22:02.234Z

renewInterval is not cleared on re-entry to startGmailWatcher, leaking the previous timer. Each config reload adds another interval that fires independently. Clear existing watcher state before starting a new one. Signed-off-by: Sebastien Tardif <[email protected]>

…er against stale child respawn

…ess to prevent late-exit respawn

Signed-off-by: Sebastien Tardif <[email protected]>

steipete · 2026-05-25T22:06:29Z

Maintainer proof before landing:

Behavior addressed: Gmail watcher re-entry clears the previous renewal interval so repeated starts do not leak timer/process lifecycle state.
Real environment tested: local Node 24.15.0 focused watcher lifecycle tests.
Exact steps or command run after this patch: CI=1 OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=120000 fnm exec --using v24.15.0 -- node scripts/run-vitest.mjs run src/hooks/gmail-watcher.test.ts --reporter=dot --pool=forks --no-file-parallelism
Evidence after fix: 1 file, 8 tests passed.
Observed result after fix: double-start, interval cleanup, stubborn child cleanup, and respawn cancel paths are covered.
Additional review: autoreview clean; GitHub CI green, including rerun CodeQL Security High (actions), at e7a74be2952ac26734560338c4566a443e6f33fa.

openclaw-barnacle Bot added size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels May 17, 2026

clawsweeper Bot added the P2 Normal backlog priority with limited blast radius. label May 17, 2026

SebTardif added a commit to SebTardif/openclaw that referenced this pull request May 20, 2026

test: add regression test for Gmail watcher interval cleanup on re-en…

1251e56

…try (openclaw#82947)

openclaw-barnacle Bot added size: S and removed size: XS labels May 20, 2026

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 20, 2026

SebTardif force-pushed the fix/gmail-watcher-interval-leak branch from 1251e56 to 4daa643 Compare May 21, 2026 18:26

openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. and removed proof: supplied External PR includes structured after-fix real behavior proof. labels May 21, 2026

clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels May 21, 2026

SebTardif force-pushed the fix/gmail-watcher-interval-leak branch 2 times, most recently from 344d9df to 800dbda Compare May 21, 2026 19:45

clawsweeper Bot added the merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. label May 21, 2026

SebTardif force-pushed the fix/gmail-watcher-interval-leak branch from 800dbda to b32a842 Compare May 21, 2026 20:07

clawsweeper Bot removed the rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. label May 21, 2026

github-actions Bot added the dependencies-changed PR changes dependency-related files label May 24, 2026

SebTardif added 4 commits May 25, 2026 21:42

fix(gmail-watcher): prevent TDZ in settleProcess and guard exit handl…

09d6c07

…er against stale child respawn

fix(gmail-watcher): strip listeners from old process after settleProc…

0f3504e

…ess to prevent late-exit respawn

retrigger CI

e7a74be

Signed-off-by: Sebastien Tardif <[email protected]>

clawsweeper Bot mentioned this pull request Jun 19, 2026

[Bug]: Multiple critical failures after updating to 2026.5.12 on macOS (Gmail sending, Dropbox terminal access, PDF generation) #81934

Closed

clawsweeper Bot mentioned this pull request Jul 6, 2026

fix(hooks): suppress unhandled stdout/stderr stream errors in gmail watcher #100519

Merged

This was referenced Jul 13, 2026

fix(gmail): preserve bind errors across watcher output chunks #106314

Merged

fix(gmail): prevent duplicate watch renewals during stalls #108974

Merged

Uh oh!

Uh oh!

Conversation

SebTardif commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Real behavior proof

Uh oh!

clawsweeper Bot commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented May 20, 2026

Uh oh!

SebTardif commented May 21, 2026

Uh oh!

clawsweeper Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SebTardif commented May 21, 2026

Uh oh!

clawsweeper Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SebTardif commented May 21, 2026

Uh oh!

clawsweeper Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SebTardif commented May 24, 2026

Update: session-utils changes removed

Uh oh!

clawsweeper Bot commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented May 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SebTardif commented May 17, 2026 •

edited

Loading

clawsweeper Bot commented May 17, 2026 •

edited

Loading

clawsweeper Bot commented May 21, 2026 •

edited

Loading

clawsweeper Bot commented May 21, 2026 •

edited

Loading

clawsweeper Bot commented May 21, 2026 •

edited

Loading

clawsweeper Bot commented May 24, 2026 •

edited

Loading

clawsweeper Bot commented May 25, 2026 •

edited

Loading