← Back to PRs

#20395: fix(googlechat): prevent infinite auto-restart and ambiguous-target 401 (fixes #20222)

by ggalmeida0 open 2026-02-18 21:24 View on GitHub →
channel: googlechat size: S
## Problem `startAccount` for the Google Chat webhook channel returned immediately after registering the passive webhook handler, causing the health monitor to treat it as "stopped" and trigger exponential-backoff restarts (5 s, 11 s, 22 s, …). Each restart appended a duplicate `WebhookTarget` entry without removing the previous one. The ambiguous-routing security check added in 2026.2.15 then rejected every inbound Google Chat request with 401 "ambiguous webhook target" once two or more stale registrations accumulated. ## Fix 1. **channel.ts** — `startAccount` now awaits the abort signal after registering the webhook, keeping the Promise pending for the lifetime of the account. The health monitor no longer sees an immediate return as "stopped". 2. **monitor.ts** — `registerGoogleChatWebhookTarget` strips any prior registration for the same `accountId` on the same path before appending, preventing stale entries from accumulating in the module-level `webhookTargets` Map. ## Tests - Added regression test: registering the same account twice (simulated restart) then sending a request yields 200, not 401. - Existing routing tests (ambiguous multi-account scenario still returns 401, single-verified-target scenario returns 200) unchanged and passing. - `pnpm build && pnpm check && pnpm test` passes (725 test files, 6027 tests). ## AI Disclosure - [x] Mark as AI-assisted in the PR title or description - [x] Note the degree of testing: **fully tested** (all Vitest suites pass locally) - [x] Session log: generated in a single Claude Code session from issue #20222 - [x] I confirm understanding of what this code does: the primary fix keeps the `startAccount` Promise pending via an abort-signal listener so the health monitor doesn't misinterpret passive webhook registration as a crash; the defensive fix deduplicates the `webhookTargets` Map by `accountId` before each registration to prevent stale entries even if the primary guard is bypassed 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes an infinite auto-restart loop and ambiguous-target 401 errors in the Google Chat webhook channel (#20222). Two complementary changes are made: - **`channel.ts`**: `startAccount` now awaits the abort signal after registering the webhook handler, keeping the Promise pending for the account's lifetime. This prevents the health monitor from treating the immediate return as "stopped" and triggering exponential-backoff restarts. - **`monitor.ts`**: `registerGoogleChatWebhookTarget` now evicts any prior registration for the same `accountId` before appending, preventing duplicate entries in the `webhookTargets` Map that would trigger the ambiguous-routing 401 rejection. - A regression test verifies that re-registering the same account (simulating a restart) does not produce an ambiguous-target 401. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with minimal risk; both fixes are well-targeted and tested. - The two changes are narrowly scoped and directly address the root cause. The abort-signal await pattern matches existing conventions (Slack uses the same pattern). The deduplication logic correctly uses `normalizeWebhookPath` for consistent key lookup and safely handles edge cases. A targeted regression test covers the primary failure scenario. No behavioral regressions are introduced to existing tests. - No files require special attention. <sub>Last reviewed commit: e1a8e33</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs