fix(googlechat): bound control and media requests#102227
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 9:44 AM ET / 13:44 UTC. Summary PR surface: Source +44, Tests +94, Docs +1. Total +139 across 3 files. Reproducibility: yes. source inspection gives a high-confidence reproduction path: current main routes Google Chat API calls through Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the split-deadline implementation after exact-head CI is green or the unrelated i18n failure is explicitly cleared, keeping short control deadlines and size-aware media budgets. Do we have a high-confidence way to reproduce the issue? Yes, source inspection gives a high-confidence reproduction path: current main routes Google Chat API calls through Is this the best way to solve the issue? Yes: the shared Google Chat response helper is the right owner boundary, and the final split between control deadlines, size-aware media budgets, and idle body guards addresses the earlier overbroad global timeout. Per-caller duplicated timeout logic would be more fragile. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f6b990124353. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +44, Tests +94, Docs +1. Total +139 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (3 earlier review cycles)
|
7ca53fe to
1a86c03
Compare
|
Land-ready maintainer rewrite at
Exact-head hosted CI: https://github.com/openclaw/openclaw/actions/runs/29020719649 (still running at this update). |
|
CI follow-up: the Google Chat change remains land-ready. The only failing job is the unrelated native-app i18n inventory assertion in This is current-
The one-line baseline repair is isolated in #102857. Per the maintainer-requested unrelated-CI exception for this landing batch, I will not mix it into this Google Chat PR. |
|
Merged via squash.
|
* fix(googlechat): add 30 s request timeout to withGoogleChatResponse * fix(googlechat): satisfy ResolvedGoogleChatAccount in api.test.ts * fix(googlechat): bound control and media requests Co-authored-by: NIO <[email protected]> --------- Co-authored-by: NIO <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
What Problem This Solves
Outbound Google Chat requests had no owner-level deadline. A stalled Google endpoint, proxy, or response body could therefore hold a Gateway request lane indefinitely.
Why This Change Was Made
All Google Chat REST operations already share
withGoogleChatResponse, so that remains the single deadline owner. The maintainer rewrite separates request classes instead of imposing one 30-second deadline on everything:This distinction matters because Google documents Chat uploads up to 200 MB: https://developers.google.com/workspace/chat/api/reference/rest/v1/media/upload
User Impact
Stalled Google Chat sends, reactions, probes, uploads, downloads, and response bodies now terminate and release their request lane. Legitimate attachments retain substantially more than 30 seconds to transfer, so the timeout fix does not regress slower or larger media operations.
Evidence
Prepared head:
1a86c03fb9e528ab759e9c95d57af83aef23cef0tbx_01kx3ejrk9vpfphhtfr48t0c95:pnpm test extensions/googlechat/src/targets.test.ts— 33/33 passed on the final rebased tree.check:changed— extension/test typechecks, lint, dependency and package guards, database-first guard, media-helper roundtrip, and import-cycle check passed.fetchWithSsrFGuardaborted a non-responsive loopback HTTP server in 134 ms against a 100 ms deadline.chat.googleapis.comreturned the expected unauthenticated401through the production guarded-fetch boundary.git diff --check— clean.Authenticated send/upload proof was unavailable because the Testbox has neither
GOOGLE_CHAT_SERVICE_ACCOUNTnorGOOGLE_CHAT_SERVICE_ACCOUNT_FILE; no credential value was inspected.