fix(gchat): explicit bot identity, media-API downloads, same-space message ids, native forward pagination (#223)

[patrick-chinchill]

Summary

Remaining Google Chat hardening from chat@4.38–4.41 (#223):

1. Explicit bot identity. New GoogleChatAdapterConfig.bot_user_id (keyword-only, [4.41/G1] Google Chat: bind webhook JWT verification to configured identity #222 convention) / GOOGLE_CHAT_BOT_USER_ID (config wins, is not None). The id is no longer learned from the first BOT mention, and gchat:botUserId is never read or written. The learning code and its _pin_task state write are gone, so no un-awaited coroutine is left behind. initialize() warns once when the id is unset. _is_message_from_self fails closed: with no id, every BOT sender counts as self. _normalize_bot_mentions rewrites only annotations whose userMention.user.name == bot_user_id and keeps the reverse-startIndex order.
2. Media-API-only downloads. fetch_data exists only when attachmentDataRef.resourceName is set, and downloadUri stays in url for display only. Errors go through _handle_google_chat_error(error, "fetchAttachmentData"), so a 429 becomes AdapterRateLimitError. rehydrate_attachment without resourceName returns the attachment unchanged. _is_trusted_gchat_download_url is deleted.
3. Same-space message ids. thread_utils.parse_message_name() uses upstream's pattern with re.fullmatch and returns a GoogleChatMessageName(space_name, message_id) NamedTuple. _assert_message_in_space runs first in edit_message, delete_message, add_reaction and remove_reaction. New fetch_message(thread_id, message_id) -> Message | None does the same check, returns None on 404, and parses the message with the thread Google reports.
4. Bounded forward pagination. _fetch_messages_forward now makes a single messages.list call with pageSize=str(min(max(limit, 1), 1000)), orderBy="createTime asc", pageToken=cursor (only when the cursor is not None) and the thread filter. next_cursor is nextPageToken.

Upstream commits mapped

Upstream	chat@	Python
2f40a322 fix(gchat): send alt=media (#801)	4.38.0	Already present; its test is ported (test_uses_media_download_api_when_attachment_data_ref_is_present asserts the ?alt=media URL)
32687038 fix(gchat): use media api for attachment downloads (#830)	4.38.1	_create_attachment, _build_gchat_fetch_data, rehydrate_attachment
f485255b fix(adapters): harden webhook tenant isolation (#877), Google Chat parts only	4.40.0	types.py bot_user_id, constructor, initialize, _normalize_bot_mentions, _is_message_from_self, _fetch_messages_forward (webhook body logging was #187)
d6343460 fix(gchat): reject message ids from another space (#938)	4.41.0	thread_utils.parse_message_name, _assert_message_in_space, fetch_message

Tests ported (from packages/adapter-gchat/src/index.test.ts and thread-utils.test.ts)

• constructor / initialization: configured id instead of persisted state; untrusted learned id not restored. env var: GOOGLE_CHAT_BOT_USER_ID resolution. These are in tests/test_gchat_comprehensive.py and replace test_restore_bot_user_id_from_state and test_does_not_overwrite_existing_bot_user_id.
• parseMessage (tests/test_google_chat_adapter.py::TestMediaDownload / TestRehydrateAttachment): media API used; fetchData with only attachmentDataRef; downloadUri not fetched when media.download fails; 429 → AdapterRateLimitError; no fetchData with only downloadUri; no rehydrate from a download URL. The "neither resourceName nor downloadUri" test in comprehensive now asserts fetch_data is None.
• normalizeBotMentions / isMessageFromSelf (tests/test_gchat_webhook.py): not learned from inbound annotations; mentions of other bots ignored; only this app normalized when another bot is mentioned first; fails closed for BOT senders when the id is unknown. The learn/persist tests in test_gchat_api.py are flipped to "no gchat:botUserId state write" and "a mentioned bot does not become self".
• message space check / fetchMessage (tests/test_gchat_api.py::TestMessageSpaceCheck / TestFetchMessage): all four methods reject a foreign space without an API call; the malformed-id it.each (9 cases); the happy path; the three fetchMessage tests. Python-only extras: exact (not prefix) space comparison, threaded and DM thread ids, and non-404 errors going through the handler.
• fetchMessages (forward): one bounded page, and cursor-based pagination (the old test_fetch_forward / test_fetch_forward_with_cursor are replaced). Python-only extra: page-size clamping.
• parseMessageName (tests/test_google_chat_adapter.py::TestParseMessageName): server-assigned name, client-assigned name, rejects %j (14 cases). Python-only sweep: trailing \n, non-ASCII letters and digits, leading or trailing dot, non-str input.

Mutation-checked: disabling each of these makes at least one test fail: the space check, the dot-segment and ? rejection, the fail-closed return, the mention id filter, the resourceName-only fetch_data, orderBy, and the 404 → None path.

Fidelity

Adapter tests are not fidelity-mapped, so the target report does not change and scripts/fidelity_target.json is not modified:

Delta vs committed report (HEAD): missing 282 -> 282 (+0)

Strict at the pin: 733/733 (730 real + 3 absorbers).

Divergences (3, within budget; rows in docs/UPSTREAM_SYNC.md, code breadcrumbs, pinned by tests)

1. Media resourceName validation (issue-mandated). Upstream hands the value to the googleapis client, which encodes it. Python interpolates it into /v1/media/{resourceName}?alt=media, so a non-string or empty value, ?, #, %, backslash, anything outside printable ASCII, and . / .. segments raise ValidationError before a token is minted.
2. Bot-identity edge cases. An empty bot_user_id counts as unset. With no id, nothing is rewritten, whereas upstream's undefined !== undefined would rewrite a BOT annotation that has no user.name. The "not configured" warning is logged once per adapter instead of on every initialize().
3. Forward limit=0 (review follow-up). limit resolves with is not None, following the repo-wide port rule, so the upstream clamp sends pageSize=1. Upstream's options.limit || 100 sends 100.

docs/UPSTREAM_SYNC.md: Google Chat is removed from the rehydrate_attachment URL-allowlist row (it no longer fetches any URL), and the three rows above are added. docs/SECURITY.md gains identity, download and message-id notes.

Consumer impact (Google Chat only; none for Slack/Teams)

• Breaking: set bot_user_id / GOOGLE_CHAT_BOT_USER_ID to the app's users/... name. Without it, other bots' messages are ignored and @-mentions of the app are no longer rewritten to @{user_name}, so default mention detection may stop matching.
• Breaking: persisted forward cursors (message names) are invalid; the API rejects them. Restart iteration without a cursor. Backward cursors are unchanged.
• Breaking: attachments without attachmentDataRef.resourceName have no fetch_data. Media failures raise the adapter's Google API error (with an HTTP code) instead of NetworkError.
• Breaking: bare or foreign-space message ids passed to edit/delete/reaction calls raise ValidationError. Ids the adapter returns are already full names.
• New: GoogleChatAdapter.fetch_message, thread_utils.parse_message_name.

Validation

ruff check / format, audit_test_quality.py (0 hard failures), --check-docs, --strict (733/733), pytest 5635 passed, 13 skipped, pyrefly 0 errors.

Closes #223
Part of #184

Merge gate

Final HEAD: fe3c22b.

Independent review (4 findings; 3 distinct issues)

• Fixed: GOOGLE_CHAT_BOT_USER_ID leaked from the shell into the "no bot id" tests (6 failures with it exported; 2 reviewers reported it). A new autouse fixture in tests/conftest.py clears it for every test. GOOGLE_CHAT_BOT_USER_ID=users/ENVBOT pytest -k 'gchat or google_chat' now gives 532 passed (was 6 failed). The env-resolution tests use monkeypatch.setenv instead of saving and restoring os.environ by hand.
• Fixed: nothing pinned the is not None precedence. New test TestConstructorEnvVarResolution::test_empty_configured_bot_user_id_does_not_fall_back_to_env_var: config bot_user_id="" plus the env var set gives None. It fails under the config.bot_user_id or env mutation. The test is cited in the "bot identity edge cases" divergence row.
• Fixed by documenting it: forward limit=0 sends pageSize=1, where upstream's limit || 100 sends 100. The code is unchanged because the repo-wide port rule keeps limit=0 (all adapters use is not None, see CHANGELOG "limit=0 no longer silently replaced by defaults"). There is a new divergence row in docs/UPSTREAM_SYNC.md and a bullet under the CHANGELOG Python-specific section, and the existing (0, "1") case pins the behavior.

gpt-6-astra: 2 rounds. Final verdict on fe3c22b: PASS, no actionable findings.

• Round 1, [P2] "Honor user impersonation in single-message fetches": not changed, because this matches upstream. Upstream fetchMessage calls this.chatApi.spaces.messages.get (app auth). Only fetchMessages/fetchChannelMessages/listThreads use impersonatedChatApi || chatApi. Switching to delegated credentials would diverge from upstream and let a message read before acting as the app see what the delegated user can see. To stop the same misreading, the docstring now explains this and test_reads_with_app_credentials_even_when_impersonation_is_configured pins it.
• Round 2: clean.

Bots: CodeRabbit is rate-limited and left no review. There were no gemini comments and no inline review comments.

CI: all green on fe3c22b: Lint & Type Check (ruff, audit, pin/doc drift, strict fidelity, pyrefly), test 3.12/3.13, CodeQL.

Local full validation: ruff check/format OK, audit 0 hard failures, --check-docs OK, strict fidelity 733/733, pytest 5637 passed / 13 skipped, pyrefly 0 errors. --report-target: missing 282 -> 282 (+0).

[coderabbitai]

Warning

Review limit reached

Next included review available in 38 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 3678fc3c-f1ab-48c9-909d-719a7db2f197

📥 Commits

Reviewing files that changed from the base of the PR and between 0cec634 and 2c51cfa.

📒 Files selected for processing (11)

• CHANGELOG.md
• docs/SECURITY.md
• docs/UPSTREAM_SYNC.md
• src/chat_sdk/adapters/google_chat/adapter.py
• src/chat_sdk/adapters/google_chat/thread_utils.py
• src/chat_sdk/adapters/google_chat/types.py
• tests/conftest.py
• tests/test_gchat_api.py
• tests/test_gchat_comprehensive.py
• tests/test_gchat_webhook.py
• tests/test_google_chat_adapter.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[patrick-chinchill]

Merge gate: CI green (test (3.12), test (3.13), Lint & Type Check, CodeQL, Analyze (python), Analyze (actions)); local Codex review (gpt-6-astra, xhigh, --base origin/main) on 2c51cfa: "No actionable regressions were found against the specified merge base. Validation passed: 5,835 tests passed, 24 skipped, and lint checks passed for the affected code and tests."; 1 astra round after merging origin/main (#193/#252/#240/#202; only conflict was tests/conftest.py autouse env fixtures, both kept), on top of the earlier clean verdict on fe3c22b; CodeRabbit rate-limited (no findings). Merging with --admin (Protect Main requires a code-owner approval).