Skip to content

feat(googlechat): keyless ADC auth + send-once for the unified adapter - #1512

Open
sebastian-hsu wants to merge 1 commit into
openabdev:mainfrom
sebastian-hsu:feat/googlechat-adc-pr
Open

feat(googlechat): keyless ADC auth + send-once for the unified adapter#1512
sebastian-hsu wants to merge 1 commit into
openabdev:mainfrom
sebastian-hsu:feat/googlechat-adc-pr

Conversation

@sebastian-hsu

@sebastian-hsu sebastian-hsu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What problem does this solve?

The Google Chat gateway adapter can authenticate to the Chat API in only two ways today: a service-account JSON key mounted into the pod (saKeyJson), or a static pre-minted accessToken. Both mean handling a long-lived secret — the key file has to be created, mounted, rotated and protected; the static token expires and must be refreshed by hand.

For a bot that already runs on GCP as its own Chat-app service account (GCE / GKE Workload Identity), that key file is redundant attack surface: the workload can already prove it is the service account. This PR adds a third, keyless option — mint the chat.bot token from the pod's own GCP identity via the metadata server + IAM Credentials self-impersonation, with no key file to mount or leak.

It also corrects the Google Chat streaming declaration: the API has no usable per-token streaming, so the adapter now declares send-once instead of attempting a post-then-edit loop. The decisive reason is structural — the unified adapter's synthetic message id is not a valid resource name, so spaces.messages.patch rejects it with 400 INVALID_ARGUMENT before any edit applies; the documented 1 write/sec-per-space quota is a further constraint.

Closes #

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491365158868619404/1542421126976643112

Review Contract

Goal

Let a Google Chat bot running on GCP authenticate to the Chat API without a service-account key file, by minting a chat.bot-scoped token from its own GCP identity (ADC → GCE metadata server → IAM Credentials generateAccessToken, the service account impersonating itself). Also declare Google Chat as send-once so core stops attempting cosmetic edits the API rejects.

Non-goals

  • Not changing or removing the existing saKeyJson and static-accessToken paths — they stay, and keep precedence over ADC.
  • Not adding keyless auth for any other platform/adapter in this PR.
  • Not implementing real streaming for Google Chat (the write rate limit makes per-token editing impractical).
  • Not managing GCP IAM setup — the operator grants roles/iam.serviceAccountTokenCreator and enables the IAM Credentials API.

Accepted Residual Risks

  • GCP-only: the ADC path needs a reachable GCE metadata server (GCE / GKE). Off-GCP it fails; the operator must fall back to saKeyJson / accessToken. Documented in docs/google-chat.md, and it is opt-in (use_adc = false by default).
  • Requires self-impersonation IAM: the SA needs serviceAccountTokenCreator over itself and the IAM Credentials API enabled. A misconfiguration surfaces as an explicit token-mint error on the first send, not a silent failure.
  • Token freshness: the minted token is cached with a 300 s refresh margin (3600 s lifetime); a metadata / IAM Credentials outage blocks outbound until it recovers — the same failure class as the existing key/static-token paths.

Acceptance Criteria

  • With GOOGLE_CHAT_USE_ADC=true on a GCP workload running as the Chat-app SA (no key file mounted), the adapter mints a chat.bot token and posts a message successfully.
  • Auth precedence holds in get_token: saKeyJson (if set) > ADC (metadata) > static accessToken.
  • Google Chat is in NON_STREAMING_PLATFORMS; resolve_streaming forces send-once on both the embedded-dispatch and the WebSocket gateway paths (no post-then-edit).
  • platform-schema conformance passes: every source ref in docs/platforms/schema/googlechat.toml resolves to a real symbol.
  • cargo clippy --workspace and --features unified are clean; cargo test passes.

Follow-ups

  • MetadataTokenSource is general enough that keyless / workload-identity auth for other Google-API-backed adapters could reuse it — deferred, non-blocking.
  • The refresh margin (300 s) and token lifetime (3600 s) are currently fixed; making them configurable is a possible later hardening if a deployment needs it.

At a Glance

Outbound to Google Chat — token source (get_token precedence)

  saKeyJson set? ──yes──▶ SA-key JWT-bearer exchange ──▶ token_cache
      │no
      ▼
  use_adc = true? ──yes──▶ MetadataTokenSource
      │no                     │
      ▼                       │ 1. GET metadata server: default SA email + base token
  static accessToken          │ 2. IAM Credentials generateAccessToken
                              │    (SA impersonates ITSELF, scope = chat.bot)
                              │ 3. cache token (3600 s, 300 s refresh margin)
                              ▼
                         chat.bot access token ──▶ Google Chat REST API

Streaming: googlechat ∈ NON_STREAMING_PLATFORMS
  resolve_streaming(platform, adapter_prefers) ──▶ send-once
    · embedded dispatch path  ──▶ compute full reply, POST once
    · WebSocket gateway path  ──▶ same
  (no post-then-edit: synthetic unified_<hex> id → patch 400 INVALID_ARGUMENT;
   1/sec-per-space quota is a further documented constraint)

Prior Art & Industry Research

Scope: how comparable open-source agent gateways authenticate their Google Chat bot identity — downloaded service-account (SA) JSON key file vs. keyless (ADC / workload identity / self-impersonation) — and how they obtain/refresh the chat.bot access token. Both reference projects were read at main as of 2026-08-27; links point at file paths (line numbers drift).

OpenClaw (openclaw/openclaw) — largest open-source AI agent gateway

Google Chat support: yes, first-party plugin @openclaw/googlechat (HTTP webhook inbound, no Pub/Sub).

How they authenticate: downloaded SA JSON key file only — no keyless/ADC path. The channel doc's setup is literally "Create a Service Account … Keys → Add Key → JSON", and the plugin "authenticates exclusively as a service account with the chat.bot scope." Inbound verifies the request's Authorization: Bearer ID token against a configured audience.

  • Evidence: docs/channels/googlechat.md. Grepping that surface for adc|application default|workload|metadata|impersonat|keyless returns zero hits — keyless is genuinely absent, not just undocumented.

What we learn: even the largest gateway ships the long-lived-key pattern openab is moving away from → keyless is a real differentiator. Their per-space serialized outbound queue + write quotas corroborate our send-once decision.

Hermes Agent (NousResearch/hermes-agent) — self-hosted agent, 27+ platforms

Google Chat support: yes, plugins/platforms/google_chat/ (Pub/Sub-pull inbound + REST outbound).

How they authenticate: documented path is a downloaded SA JSON key (GOOGLE_CHAT_SERVICE_ACCOUNT_JSON), but the adapter code also supports keyless ADC as a fallback. _load_sa_credentials() priority: (1) explicit service_account_json, (2) GOOGLE_APPLICATION_CREDENTIALS, (3) google.auth.default() — ADC on Cloud Run / GCE / GKE with an attached workload identity, scopes chat.bot + pubsub.

  • Evidence: plugins/platforms/google_chat/adapter.py (_load_sa_credentials), doc google_chat.md.
  • Nuance: Hermes' keyless mode is plain google.auth.default() — it uses the attached SA's own metadata token directly. It does not call IAM Credentials generateAccessToken to self-impersonate, so the attached SA must itself carry chat.bot.

What we learn: closest prior art; confirms google.auth.default() on GCE/GKE as the sanctioned keyless entry point. openab's design is a stricter, more explicit superset — see the comparison and §Why.

Other industry practice

Comparison

Project Google Chat Auth model How the bot token is obtained
OpenClaw Yes (HTTP webhook) Key file only — SA JSON, chat.bot; no keyless path Google client libs mint/refresh from the SA private key
Hermes Agent Yes (Pub/Sub + REST) Key file (docs) + keyless ADC fallback (code) explicit JSON → GOOGLE_APPLICATION_CREDENTIALSplain google.auth.default() (attached SA's own token); no self-impersonation
Google (official) recommends keyless / short-lived; SA key = last resort ADC + workload identity; short-lived tokens via generateAccessToken
openab (this PR) Yes Keyless — ADC + metadata + IAM generateAccessToken self-impersonation ADC base identity from metadata → self-impersonate the Chat-bot SA → short-lived chat.bot token; auto-refresh, no key on disk

Proposed Solution

Keyless ADC outbound token (MetadataTokenSource, crates/openab-gateway/src/adapters/googlechat.rs). A new token source that, when use_adc is set:

  1. reads the default service-account email and a base access token from the GCE metadata server;
  2. calls IAM Credentials generateAccessToken so the service account impersonates itself, scoped to https://www.googleapis.com/auth/chat.bot;
  3. caches the result behind an RwLock with a double-checked refresh, using the same 300 s margin as the existing paths (ttl_from_expire_time clamps the lifetime to [0, 3600]).

get_token gains a strict precedence: token_cache (SA-key exchange) → metadata_source (ADC) → static access_token. Wiring: GoogleChatConfig.use_adc (config.rs), plumbed through GatewayGoogleChatConfig and the three from_parts call sites (lib.rs, src/main.rs), surfaced as GOOGLE_CHAT_USE_ADC and the chart's gateway.googleChat.useAdc.

Send-once streaming. NON_STREAMING_PLATFORMS (renamed from NON_EDITABLE_PLATFORMS, gateway.rs) now gates a new resolve_streaming(platform, adapter_prefers_streaming) (adapter.rs) used by the embedded stream_prompt_blocks path, mirroring what platform_supports_streaming already did for the WebSocket path. Google Chat therefore never attempts a post-then-edit loop. The schema (googlechat.toml) is updated from partial to not_implemented with the rationale, and its source refs point at the real symbols.

Why this approach?

Weighed against the alternatives from the research:

  • vs. OpenClaw / Hermes' documented key-file path. A downloaded SA JSON is a long-lived secret (Google notes such keys can be valid for years) that must be stored, mounted, rotated, and kept out of logs / backups / images — a standing exfiltration target. Removing it shrinks the on-disk credential surface to zero and eliminates key rotation, matching Google's explicit "avoid SA keys" guidance. This is the primary driver.
  • vs. Hermes' plain-ADC (google.auth.default()) keyless fallback. Plain ADC forces the node / workload identity to be the Chat bot — it must carry chat.bot directly. Self-impersonation decouples the runtime identity from the bot identity: the node keeps a minimal identity and is granted roles/iam.serviceAccountTokenCreator on a dedicated Chat-bot SA, and we request precisely the chat.bot scope via generateAccessToken regardless of the node SA's default scopes. Cleaner for least-privilege and multi-tenant separation; tokens are short-lived and auto-refresh with nothing persisted. Crucially, chat.bot is a Workspace scope and is not a subset of cloud-platform, so no ?scopes= parameter on the metadata token can produce it — generateAccessToken self-impersonation is therefore not merely cleaner here, it is the only viable keyless way to obtain a chat.bot token.

Accepted trade-offs (honest costs):

  1. Environment coupling — works only where a metadata server + ADC exist (GCE / GKE / Cloud Run). Off-GCP there is no metadata identity, so the SA-key path stays as an explicit fallback (not a default), mirroring Hermes' priority chain.
  2. IAM prerequisite — the runtime SA must hold roles/iam.serviceAccountTokenCreator on the target/self SA, plus one extra hop to the IAM Credentials API per token mint. This is a deliberate, auditable IAM binding replacing an unauditable file on disk.

The feature is opt-in (use_adc = false by default); the key-file and static-token paths are untouched for everyone else.

Alternatives Considered

  • Plain ADC (google.auth.default()), like Hermes Agent's keyless fallback. Simpler, but it makes the node/workload identity be the bot (the attached SA must carry chat.bot). We chose generateAccessToken self-impersonation instead to decouple the runtime identity from the bot identity and request exactly the chat.bot scope — see §Why.
  • Keep only the SA-key JSON file. Rejected as the default for GCP-hosted bots: a long-lived secret that must be mounted, protected and rotated, when the workload can already prove its identity. Retained as a fallback.
  • Static pre-minted accessToken. Kept as a fallback, but not a fix: it expires and needs out-of-band refresh.
  • Full Workload Identity Federation (off-GCP → GCP). Out of scope — this feature targets workloads already running on GCP as the Chat-app SA; WIF for external identities is a larger, separate design.
  • Attempt cosmetic streaming with rate-limit backoff. Rejected: the unified adapter's synthetic unified_<hex> id is not a valid resource name, so patch rejects it with 400 INVALID_ARGUMENT before any edit — per-token editing cannot work at all; the documented 1 write/sec-per-space quota is a further constraint. Send-once is what the API actually supports.

Validation

Run locally on stable pinned to the CI toolchain (2026-07-13 ≈ 1.97.0):

Rust:

  • cargo clippy --workspace -- -D warnings — clean
  • cargo clippy --workspace --features unified -- -D warnings — clean
  • cargo test -p openab-gateway passes, incl. config_first_conformance (every_platform_env_var_has_a_config_section_field)
  • platform-schema conformance — all four source refs in googlechat.toml verified to resolve (gateway.rs#NON_STREAMING_PLATFORMS, adapter.rs#resolve_streaming, adapter.rs#uses_native_streaming, googlechat.rs#MetadataTokenSource). The crate's own test wasn't run locally (my .claude/worktrees/ checkout nests under the repo, which confuses cargo's standalone-workspace resolution — a local artifact only); CI runs it on a flat checkout.

Helm (chart touched — gateway.yaml, values.yaml):

  • Change is one additive conditional (useAdcGOOGLE_CHAT_USE_ADC env, plus useAdc in the $hasGoogleChat guard). No existing charts/openab/tests/*_test.yaml asserts the gateway Google Chat env, so it does not disturb current unittests; helm unittest charts/openab runs on CI (helm not installed locally).

Manual:

  • On a GKE workload running as the Chat-app SA with GOOGLE_CHAT_USE_ADC=true and no key file: the bot mints a chat.bot token and replies in a space; confirm precedence by additionally setting saKeyJson and observing the key path win.

Note: my machine's default stable is 1.98, whose newer useless_format / doc_lazy_continuation lints flag pre-existing code (e.g. setup/wizard.rs) that the CI toolchain does not; the doc-comment lint on the new resolve_streaming doc was the one real hit and is fixed in this branch. The unrelated openab-core secrets::tests::resolve_exec_nonzero_exit failure is a pre-existing, environment-specific subprocess-error-string assertion (that file is not touched by this PR).

@openab-app openab-app Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Aug 27, 2026
@sebastian-hsu
sebastian-hsu force-pushed the feat/googlechat-adc-pr branch from 639b61f to f7baf96 Compare August 27, 2026 06:28
@openab-app openab-app Bot removed the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Aug 27, 2026

@canyugs canyugs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 1 review — verified by live deployment, not just reading

I ran the auth flow against the real GCP and Google Chat APIs rather than only reading the diff, because the novel part of this PR is an interaction with two external services that unit tests can only assert against the author's own assumptions. All identifiers below are redacted; no token values are reproduced.

The keyless ADC design works and I recommend it lands. I posted a real message to a real space with a token minted entirely keylessly:

POST /v1/projects/-/serviceAccounts/<sa>:generateAccessToken
  → HTTP 200, expireTime 2026-08-27T15:34:25Z
tokeninfo on the minted token
  → scope: https://www.googleapis.com/auth/chat.bot   (and nothing else)
POST /v1/spaces/<space>/messages
  → HTTP 200, sender.type = BOT

That third line is the important one. The base token was cloud-platform-scoped and the minted token carries only chat.bot. This empirically validates the central argument in §Why for generateAccessToken self-impersonation over Hermes' plain google.auth.default() — the scope really is narrowed, and the runtime identity really is decoupled from the bot identity. Worth noting that the official docs make this argument even stronger than the PR does: chat.bot is a Workspace scope and is not a subset of cloud-platform, so no ?scopes= parameter on the metadata server can produce it. Self-impersonation is not merely cleaner here, it is the only viable keyless path. I'd put that in §Why.

I also confirmed the explicit IAM grant in the docs is genuinely required: roles/owner does not include iam.serviceAccounts.getAccessToken, so an owner still gets IAM_PERMISSION_DENIED without the serviceAccountTokenCreator binding.

Cross-checks against official docs that all hold: the projects/-/ wildcard is required as used; iam.serviceAccounts.getAccessToken is contained in roles/iam.serviceAccountTokenCreator; accessToken/expireTime are exactly the response fields; expireTime is RFC 3339 and may carry fractional digits or a non-Z offset, both of which parse_from_rfc3339 accepts; the metadata token endpoint returns {"access_token":…,"expires_in":…,"token_type":…}, matching the parsing and the wiremock fixture. The chat.bot scope requires no admin approval, so no domain-wide delegation is needed for an impersonated token.

I verified the OpenClaw half of §Prior Art directly against a local checkout: extensions/googlechat/ has zero genuine hits for adc|application.default|workload.identity|metadata.google|impersonat|generateAccessToken|keyless (the one apparent match is readCredentialsFile matching adc case-insensitively). The claim holds. I did not verify the Hermes Agent claims.


Recommended blockers

1. Stale base — check fails on a lint this PR did not introduce

cargo clippy --workspace -- -D warnings fails at crates/openab-core/src/setup/wizard.rs:163 (useless_format). That line was fixed on main in #1511; this branch's merge base predates it. Merging origin/main clears it — I verified locally on the merged tree:

cargo clippy --workspace -- -D warnings                     → clean
cargo clippy --workspace --features unified -- -D warnings   → clean
cargo test -p openab-gateway                                 → 315 passed, 0 failed
cargo test --manifest-path crates/platform-schema/Cargo.toml  → 17 passed

main touches 6 files, none overlapping this PR, so no conflicts. Incidentally the platform-schema conformance run that §Validation lists as unverified locally does pass — all four source refs resolve.

The one openab-core failure, secrets::tests::resolve_exec_nonzero_exit, also fails on unmodified origin/main on macOS, so §Validation's characterisation of it as pre-existing and environment-specific is accurate.

The smoke-test (Dockerfile.hermes) failure is an upstream install-script download returning curl exit 22; smoke-test-unified (hermes) is green. Unrelated.

2. docs/google-chat.md Option C will fail as written on GCE

Calling generateAccessToken requires the caller's token to carry https://www.googleapis.com/auth/iam or cloud-platform. The metadata server returns a token bounded by the instance's access scopes, and GCE's default scope set contains neither. Reproduced with a base token restricted to exactly the GCE defaults, same SA and same IAM binding that succeeded above:

base scope: devstorage.read_only logging.write monitoring.write
            service.management.readonly servicecontrol trace.append

POST …:generateAccessToken
  → HTTP 403 PERMISSION_DENIED
    "Request had insufficient authentication scopes."

So the IAM prerequisites can be entirely correct and this still fails. Option C lists GKE / GCE / Cloud Run and the prerequisites cover the IAM binding and API enablement, but not the access scope. GKE Workload Identity and Cloud Run are cloud-platform-scoped and unaffected; a default-scope GCE VM is not, and VM scopes are immutable after creation (gcloud compute instances set-scopes plus a stop/start). Please add the requirement, and ideally name the 403 string so operators can match on it — it says "scopes", not "permission", which is the only signal distinguishing it from a missing IAM role.

3. The streaming rationale in googlechat.toml asserts behaviour I could not reproduce

The recorded rationale for send-once is that per-token editing "would immediately 429" under the 1 write/sec/space limit. I could not reproduce that:

8 sequential PATCH to one message (~3/s)   → 8 × HTTP 200
25 concurrent PATCH to the same message     → 25 × HTTP 200

No 429 at any point. The quota table does document per-space writes as 1/second, but enforcement is evidently burst-tolerant. Caveat: my space was a DM, and the same page notes additional internal limits that are not exposed, so I am not claiming the limit does not exist — only that "would immediately 429" is not supported by observation.

That matters because the second, structural reason is solid and is the one that actually justifies send-once. A synthetic id cannot be patched at all:

PATCH /v1/spaces/<space>/messages/unified_a1b2c3d4e5f6
  → HTTP 400 INVALID_ARGUMENT
    "Missing or malformed message resource name in the request…"
PATCH /v1/spaces/<space>/messages/<real id>
  → HTTP 200

It is 400 INVALID_ARGUMENT, not 404 — the id does not parse as a resource name, so this fails earlier than the PR describes. 404 appears in the PR body, the googlechat.toml note, and the gateway.rs comment.

The conclusion is right; send-once is correct. I'm asking for the recorded reasoning to be corrected because docs/platforms/schema/googlechat.toml is durable rationale future maintainers will cite. Suggest leading with the resource-name failure and its real status code, and citing the quota as a documented constraint rather than an asserted runtime behaviour.


Contract challenges (Round 1)

The residual-risk claim about diagnosability does not match the code. §Accepted Residual Risks states a misconfiguration "surfaces as an explicit token-mint error on the first send, not a silent failure." On the reply path, get_token() returning None yields:

info!(text = %reply.content.text,
      "googlechat reply (dry-run, no credentials configured)");
// → GatewayResponse { success: false, error: "no credentials configured" }

For the ADC case that message is wrong — credentials are configured (use_adc = true); the mint failed. The real cause is on a separate error! line from get_token. And the reply is dropped, so the Chat user sees nothing at all. So it is a silent failure from the user's side and a misleading one from the operator's side. Combined with finding 2, an operator who follows Option C onto a default-scope GCE VM gets: no reply, a log line blaming missing credentials, and the real 403 elsewhere. I'd either amend the claim or distinguish the ADC failure in that message.

The environment constraint is stated as the wrong axis. §Accepted Residual Risks frames it as GCP-only versus off-GCP. The sharper constraint is the access scope: an on-GCP GCE workload can fail too. Worth restating in those terms, since that is what an operator has to check.

Suggested Acceptance Criterion. The existing manual criterion covers the success path on GKE. Given finding 2, please also cover a default-scope GCE VM and assert the failure is diagnosable, so criterion 2 above cannot silently regress.


Non-blocking

  • from_parts comment contradicts the code. The comment says ADC is enabled "only when no SA key was resolved," but adapter.metadata_source = use_adc.then(MetadataTokenSource::new) is unconditional; only the info! is gated. Behaviour is correct because get_token short-circuits on token_cache. Related: if sa_key_file is unreadable or the JSON is invalid, both paths only warn! and ADC silently takes over — an unannounced identity switch. A warn! naming the fallback would help.
  • ADC_TOKEN_LIFETIME_SECS comment. "IAM caps impersonated tokens at 3600s" — 1 hour is the default maximum, extendable to 12 hours via constraints/iam.allowServiceAccountCredentialLifetimeExtension. The clamp is safely conservative, but the comment reads as an absolute cap.
  • expireTime fallback is unreachable. The API reference states the expiration time is always set, so .unwrap_or(ADC_TOKEN_LIFETIME_SECS) will not trigger. Harmless; the Err(_) arm inside ttl_from_expire_time is the one doing real defensive work.
  • Cache is disabled for short-lived tokens. elapsed < ttl.saturating_sub(300) means any ttl <= 300 mints on every send. Same formula as GoogleChatTokenCache, so not new, but ADC pays an extra IAM hop per mint.
  • Case-sensitive bool parsing, duplicated three times. v == "true" || v == "1" in config.rs and twice in lib.rs means GOOGLE_CHAT_USE_ADC=True silently does nothing. config.rs already has env_flag_true_one, which is case-insensitive. Consistent with the adjacent allow_all_users precedent, so it's a judgement call.
  • The env path for use_adc is untested. The resolve test only exercises use_adc: Some(true); GOOGLE_CHAT_USE_ADC is added to the remove_var list but never set and asserted — which is precisely where the previous item would bite.
  • MetadataTokenSource::{metadata_base, iam_credentials_base} are pub. Documented as test seams, but as public fields production code can retarget the exchange, and the bearer sent there is a live metadata token. A private field with a #[cfg(test)] setter would keep the seam without widening the credential path.
  • Dockerfile.claude's OPENAB_BUILD_FEATURES looks like scope creep. Nothing in the repo passes it; the comment says "the reviewer image passes googlechat" but no such caller exists here. It's also in a file AGENTS.md marks deprecated in favour of Dockerfile.unified, which already builds --features unified and therefore already includes googlechat. Suggest dropping it from this PR.
  • IAM propagation takes about 30 seconds. The serviceAccountTokenCreator binding needed roughly 30 s before impersonation succeeded. refresh() has no retry, so the first send right after a fresh deployment can fail. Fine operationally, worth a line in the docs.

One underclaimed win

resolve_streaming fixes more than Google Chat. Following the call chain, Dispatcher::stream_prompt_blocks delegates to AdapterRouter::stream_prompt_blocks, which reads adapter.use_streaming() — the unified adapter's global Telegram flag — not the platform_supports_streaming-gated local in run_gateway_adapter (that one only feeds GatewayAdapter::use_streaming on the standalone path). So in unified/embedded mode line and lineworks were also inheriting the Telegram flag and attempting edits on platforms with no edit API. This PR fixes that too. Worth stating in the description so reviewers see the real blast radius.


Summary

The ADC half is sound and I verified it end to end against live APIs. My recommended blockers are one rebase, one documentation gap that will break real GCE deployments, and one correction to durable recorded rationale — none of them defects in the ADC implementation itself. Everything else is non-blocking.

I don't have maintainer rights here, so this is a review comment rather than a formal block, and the contract freeze is @thepagent's call.

Evidence gathered against a scratch GCP project I own; the temporary human-account impersonation grant used for testing has been removed.

@thepagent

Copy link
Copy Markdown
Collaborator

CI and smoke test failing. Changing status to Draft until fixed.

@thepagent
thepagent marked this pull request as draft August 27, 2026 16:40
Keyless ADC (MetadataTokenSource): mint a chat.bot-scoped token from the
workload's own GCP identity — GCE metadata (SA email + base token) -> IAM
Credentials generateAccessToken (self-impersonation). No SA key file.
Config [googlechat].use_adc / GOOGLE_CHAT_USE_ADC; auth precedence SA key >
ADC > static token; cache under the IAM-granted expireTime (fallback 3600s).

Send-once for Google Chat: its write rate limit is 1/sec/space
(create+patch+delete combined) so per-token streaming edits 429, and the
unified adapter returns a synthetic message id that patch can't target (404).
googlechat added to NON_STREAMING_PLATFORMS (renamed from
NON_EDITABLE_PLATFORMS); resolve_streaming forces send-once on both the
embedded dispatch (stream_prompt_blocks) and WebSocket gateway paths.

Also: Dockerfile.claude OPENAB_BUILD_FEATURES arg, Helm googleChat.useAdc
value, docs + config-first conformance entry + googlechat.toml schema record.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sebastian-hsu
sebastian-hsu force-pushed the feat/googlechat-adc-pr branch from f7baf96 to 0b57f3b Compare August 28, 2026 00:29
@sebastian-hsu

Copy link
Copy Markdown
Contributor Author

Thanks @canyugs — verifying the auth flow against real GCP/Chat rather than the diff is exactly the right lens here, and the scope-narrowing evidence (a cloud-platform-scoped base token producing a chat.bot-only minted token) is the strongest possible confirmation of the §Why argument. All three blockers are addressed.

1. Stale base — clippy on pre-existing wizard.rs. Rebased onto current origin/main (now includes #1511), which clears the setup/wizard.rs:163 useless_format. Verified on the rebased tree: cargo clippy --workspace -- -D warnings and --features unified both clean, cargo test -p openab-gateway green. The openab-core secrets::tests::resolve_exec_nonzero_exit failure is pre-existing on unmodified main, as you confirmed.

2. docs/google-chat.md Option C — GCE access scope. Added the missing requirement: the metadata base token must carry cloud-platform (or .../auth/iam) scope, because generateAccessToken requires it on the caller. A default-scope GCE VM returns 403 PERMISSION_DENIED: "Request had insufficient authentication scopes." even with the IAM binding correct — I named that string and pointed out it says scopes, not permission, so operators can tell it apart from a missing role. Also noted GKE Workload Identity / Cloud Run are cloud-platform-scoped and unaffected, and that GCE scopes are immutable after creation (set-scopes + stop/start).

3. googlechat.toml streaming rationale. Corrected to lead with the decisive, reproducible reason: the synthetic unified_<hex> id is not a valid resource name, so patch rejects it with 400 INVALID_ARGUMENT — not 404, and it fails before any edit applies. The 1 write/sec-per-space quota is now cited as a documented constraint on high-frequency editing, not an asserted "would immediately 429" (your 8-sequential / 25-concurrent PATCH-all-200 result is noted — enforcement is burst-tolerant). The same 404→400 / quota-framing correction is applied in the PR body (What problem / At a Glance / Alternatives).

§Why now also carries your stronger point: chat.bot is a Workspace scope and is not a subset of cloud-platform, so no ?scopes= on the metadata token can produce it — generateAccessToken self-impersonation is the only viable keyless path, not merely the cleaner one.

On CI: the smoke-test (Dockerfile.hermes) failure is the upstream install-script curl exit 22 you identified (the -unified hermes variant is green) — unrelated to this change.

Reviewed head is now the rebased commit. Thanks again for the thoroughness.

@canyugs canyugs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 2 — fix verification

Scoped per docs/review-contract.md for later rounds: unresolved Round 1 findings, changes since the last reviewed commit, regressions from those changes, and Acceptance Criteria compliance. No new architecture discovery.

Reviewed head: 0b57f3ba (previous: f7baf967).

All three Round 1 blockers verified fixed

1. Stale base. origin/main is now an ancestor of the head, so the wizard.rs:163 useless_format from the old merge base is gone. Independently re-run on 0b57f3ba:

cargo clippy --workspace -- -D warnings                       → clean
cargo clippy --workspace --features unified -- -D warnings     → clean
cargo test -p openab-gateway                                   → 315 passed, 0 failed
cargo test --locked -p platform-schema (standalone manifest)    → 17 passed, 0 failed

The check job has flipped from fail to pass on CI, matching.

2. GCE access scope, docs/google-chat.md. Resolved, and more completely than I asked. Option C now states that the metadata base token must carry cloud-platform (or .../auth/iam) because generateAccessToken requires it on the caller; reproduces the exact 403 PERMISSION_DENIED: "Request had insufficient authentication scopes."; tells operators to match on scopes rather than permission to distinguish it from a missing role; notes GKE Workload Identity and Cloud Run satisfy it automatically; and covers the part that actually bites, that GCE scopes are immutable after creation, with the set-scopes plus stop/start remedy. That is the failure mode from Round 1 finding 2 fully described.

3. googlechat.toml streaming rationale. Resolved. The note now leads with the decisive and reproducible reason — the synthetic unified_<hex> id is not a valid resource name, so patch rejects it with 400 INVALID_ARGUMENT before any content is applied — and demotes the quota to "a documented constraint on high-frequency editing rather than an observed hard failure, since enforcement is burst-tolerant in practice." That is an accurate description of what I measured. 404 no longer appears in any Google Chat surface.

The §Why addition is a good call: stating that chat.bot is a Workspace scope and not a subset of cloud-platform, so no ?scopes= on the metadata token can produce it, turns the design rationale from a preference into a necessity.

Regression check: clean, and narrowly so

Comparing each head's net diff against its own merge base, file by file, only two files differ between f7baf967 and 0b57f3ba:

* changed   docs/google-chat.md
* changed   docs/platforms/schema/googlechat.toml
= unchanged Dockerfile.claude, charts/…/gateway.yaml, charts/…/values.yaml,
            config.toml.example, crates/openab-core/src/{adapter,config,gateway}.rs,
            crates/openab-gateway/src/adapters/googlechat.rs,
            crates/openab-gateway/src/lib.rs,
            crates/openab-gateway/tests/config_first_conformance.rs,
            docs/config-reference.md, docs/platforms/schema/lineworks.toml,
            src/main.rs

The file list is identical at both heads and the Rust delta is empty, so there is no behavioural regression surface introduced by this round. The rebase is clean with nothing smuggled in.

Unresolved from Round 1 — ORIGINAL, and I am not blocking on any of them

Recording these so they are not lost, not to reopen them. The Round 1 contract challenges and the ten non-blocking items are unchanged in 0b57f3ba. My position is that none of them meet the Late Blocker Gate and they should not hold this PR:

  • §Accepted Residual Risks still reads "A misconfiguration surfaces as an explicit token-mint error on the first send, not a silent failure," and still frames the constraint as GCP-only. Both are now narrower than what docs/google-chat.md itself says after fix 2, so the contract text and the docs disagree. Since the docs are where operators look and they are now correct, this is bookkeeping.
  • No Acceptance Criterion covers the default-scope GCE case, so fix 2 is documentation-only with no test guarding it. Worth a Follow-up entry rather than a blocker.
  • The ten non-blocking items from Round 1 stand as written, including the from_parts comment still contradicting the unconditional metadata_source assignment, the ADC_TOKEN_LIFETIME_SECS comment still describing 3600 s as a hard cap when it is the default maximum, and Dockerfile.claude's unused OPENAB_BUILD_FEATURES.

Two process notes, neither a review finding:

  • I do not see a contract freeze record from the maintainer, so Round 1 was never formally closed under the lifecycle in docs/review-contract.md. I have applied later-round scoping anyway, since the practical effect is the same.
  • The PR is still a Draft, set on the previous head over CI failures that are now fixed. That status is currently self-sustaining: docker-smoke-test.yml and docker-smoke-test-unified.yml both gate on if: github.event.pull_request.draft == false, so the smoke tests cannot run to clear the condition they were paused for. Someone with write access will need to mark it Ready.

Bottom line

Every blocker I raised in Round 1 is fixed and independently verified, this round introduced no regressions, and the only code-affecting criteria I can check locally all pass. No blockers remain from my side. The remaining items are contract wording and optional hardening, and the freeze and merge decisions are @thepagent's.

@sebastian-hsu
sebastian-hsu marked this pull request as ready for review August 28, 2026 05:56
@sebastian-hsu

Copy link
Copy Markdown
Contributor Author

Marking ready for review. All three Round 1 blockers are fixed and independently re-verified in @canyugs' Round 2: cargo clippy --workspace (and --features unified) clean on the rebased head, gateway/platform-schema tests green, and the docs (google-chat.md GCE access-scope + 403 string) and googlechat.toml rationale (patch of the synthetic id is 400 INVALID_ARGUMENT, quota framed as a documented constraint) corrected.

The docker-smoke-test / docker-smoke-test-unified jobs are gated on draft == false, so they couldn't run while this sat in Draft — moving it out of Draft lets them run. @thepagent — the contract freeze and merge decisions remain yours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants