feat(google-chat): handle card click callbacks - #36069
Conversation
e5153d6 to
289fd8c
Compare
Add _is_callback_user_authorized mirroring Telegram's contract: when the runner _is_user_authorized path is unavailable, fall back to the GOOGLE_CHAT_ALLOWED_USERS allowlist (comma-separated emails, '*' wildcard) and GATEWAY_ALLOW_ALL_USERS, denying by default instead of silently letting any card click resolve a clarify/approval. Closes the fail-open gap where a missing auth_fn let unauthorized clicks through.
mark_awaiting_text is the 'Other (type answer)' mode-flip; calling it in send_clarify captures the user's next message as the clarify response, racing the button-click path. It is already called in the __other__ branch of _dispatch_clarify_card_click — drop the send-time duplicate.
72cc52e to
ec76ec7
Compare
|
Thanks for completing the Google Chat interaction path. The underlying gap is real: current main still ACKs card/widget events without dispatching them ( Problems
Suggested changes
Automated hermes-sweeper review. |
|
Your whole stack has now landed on main: #36035 (HTTP inbound) via #65545, #36068 (clarify cards) via #65546, and #36061 (generic platform event callback route) via #65702 — all with your authorship preserved. This PR is the last piece. Could you rebase it onto current main? It should shrink to just the card-click handling now that the base commits are merged. Note #65702 added one hardening change on top of your route (verifiers run via asyncio.to_thread and fail closed on exceptions) — your callback dispatch should slot straight in. Happy to review as soon as it's rebased. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Five PRs address the Google Chat interaction stack: #36035/#65545 make HTTP inbound possible without Pub/Sub, #36061/#65702 provide authenticated callback routing, and #36069 uniquely handles the remaining cause by dispatching card-click events that were previously only ACKed. The first two changes are already represented on main by #65545 and #65702, leaving only a clean current-main extraction of #36069's card-click logic to land.
Related pull requests
- #36035 [closed]
related— (+152/-85) — superseded by merged #65545: This relaxes Pub/Sub requirements and enables HTTP-mode configuration, but its original diff provides no callback route, authentication, or dispatch handler, so it does not by itself fix dropped card clicks. It remains relevant as the source change salvaged into #65545; the contributor keep_open review correctly identified that the original slice was not operational alone. - #36061 [closed]
related— (+670/-86) — superseded by merged #65702: This adds the generic callback route, Google OIDC verification, and HTTP MESSAGE dispatch required before card clicks can enter the adapter, but it does not dispatch CARD_CLICKED events. It remains relevant because #65702 salvaged it and added the contributor-requested off-loop, fail-closed verifier hardening; its keep_open review identified configuration coherence and production-path coverage concerns rather than grounds to merge the old head unchanged. - #36069
related— (+1430/-93) — keep open, but rebase before merge: Its unique adapter diff parses card/add-on callback payloads, resolves authorized clarify choices, and forwards unknown actions, directly fixing the remaining ACK-without-dispatch cause. Consistent with the contributor keep_open review, the current stacked head must not be merged as-is because its stale API-server snapshot would regress run-lifetime and authenticated-readiness behavior; salvage only the card-click slice onto current main, preserving #65702's hardened route. - #65545 [merged]
related— (+153/-85) — merged prerequisite/reference implementation: This is the current-main salvage of #36035, making Pub/Sub optional when HTTP inbound is configured. It removes the transport-configuration blocker but intentionally does not implement callback routing or card-click dispatch. - #65702 [merged]
related— (+536/-3) — merged prerequisite/reference implementation: This is the current-main salvage of #36061, adding the generic authenticated platform callback route and Google Chat HTTP MESSAGE dispatch, with verifier execution moved off the event loop and exceptions failing closed. It supplies the ingress needed by #36069 but still leaves card/widget callbacks undispatched.
Duplicates
#36035 and #65545 are substantially the same HTTP-without-Pub/Sub change, with #65545 being the merged current-main salvage. #36061 and #65702 are substantially the same callback-routing change, with #65702 being the merged, hardened current-main salvage. #36069 contains those stacked bases but also has the unique card-click implementation.
Suggested consolidation
Merge #36069 only after rebasing or salvaging its focused card-click handling onto current main and verifying that #65702's off-loop fail-closed verifier plus current API-server run-lifetime, SSE, stop, and authenticated-readiness behavior remain intact. This follows the keep_open review on #36069 rather than overriding it: the diff demonstrates a real unique fix, while the contributor explicitly blocks merging the stale stacked head as-is. #36035 and #36061 are already closed and superseded by merged duplicates #65545 and #65702; no further duplicate PR needs closing.
Cross-PR triage: Reviewed 5 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 175 kB of PR diffs, 12 kB of issue/PR text, 7 kB of discussion (9 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
What does this PR do?
Handles authenticated Google Chat card-click callbacks, including resolving built-in
clarify()card choices and forwarding unknown card actions as structured agent-visible events.This completes the Google Chat card interaction path so card buttons can produce authenticated gateway events instead of only rendering static cards.
Depends on #36035, #36061, and #36068. This draft is intentionally stacked; after the dependencies land, this branch should be rebased so the final diff contains only card-click callback handling.
Related Google Chat PRs: #36027, #36035, #36061, #36068.
Related Issue
N/A — follow-up to the Google Chat HTTP callback and clarify-card rendering work.
Type of Change
Changes Made
plugins/platforms/google_chat/adapter.py: parse Google Chat card-click/add-on callback payloads.plugins/platforms/google_chat/adapter.py: resolvehermes_clarifybutton choices throughtools.clarify_gateway.plugins/platforms/google_chat/adapter.py: apply adapter/gateway authorization checks before resolving clarify card clicks.plugins/platforms/google_chat/adapter.py: dispatch unknown card actions to the agent as synthesized command events.tests/gateway/test_google_chat.py: cover action parsing, add-on normalization, generic card-click dispatch, clarify resolution, and unauthorized clarify-click rejection.How to Test
python -m py_compile plugins/platforms/google_chat/adapter.py tests/gateway/test_google_chat.py gateway/platforms/api_server.py gateway/run.pygit diff --checkscripts/run_tests.sh tests/gateway/test_api_server.py tests/gateway/test_google_chat.pypython /root/.codex/skills/hermes-upstream-pr/scripts/pr_preflight.py --base HEAD~1Checklist
Code
Documentation & Housekeeping
cli-config.yaml.exampleupdate N/A.CONTRIBUTING.md/AGENTS.mdupdate N/A.Related PRs
Screenshots / Logs
Validation passed locally: