feat: bridge adapters

[pedronauck]

Summary by CodeRabbit

• New Features
  • Added Discord bridge with webhook interactions, message create/edit/delete, reactions, and signature verification.
  • Added Google Chat bridge supporting direct webhooks and Pub/Sub, threading, DM policy, and delivery operations.
  • Added GitHub bridge for issue/review comment create/update/delete with PAT and App auth modes.
• Tests
  • Added extensive end-to-end and unit tests covering webhook handling, signature verification, mapping, and delivery flows.

[coderabbitai]

Walkthrough

Adds three new bridge adapters (Discord, Google Chat, GitHub): CLI entrypoints, environment-driven marker helpers, full providers with webhook HTTP servers, external-API clients, delivery/ingest sequencing, and extensive unit/integration tests.

Changes

Cohort / File(s)	Summary
Discord bridge extensions/bridges/discord/main.go, extensions/bridges/discord/markers.go, extensions/bridges/discord/provider.go, extensions/bridges/discord/provider_test.go	New Discord executable entrypoint, marker-file utilities, provider implementing webhook server, Ed25519 signature verification, inbound mapping, delivery sequencing and REST client, plus comprehensive tests.
Google Chat (gchat) bridge extensions/bridges/gchat/main.go, extensions/bridges/gchat/markers.go, extensions/bridges/gchat/provider.go, extensions/bridges/gchat/provider_test.go	New GChat executable entrypoint, marker-file utilities, provider implementing direct & Pub/Sub webhook handling, JWT bearer auth, RS256 verification, delivery sequencing and API client, plus comprehensive tests.
GitHub bridge extensions/bridges/github/api.go, extensions/bridges/github/main.go, extensions/bridges/github/markers.go, extensions/bridges/github/provider.go	New GitHub API client and interface (PAT and App auth with JWT→installation token flow), executable entrypoint, marker-file utilities, provider with HMAC webhook verification, comment create/update/delete delivery, per-instance ordering, and runtime integrations.

Sequence Diagram(s)

sequenceDiagram
    participant Client as HTTP Client
    participant Server as Bridge HTTP Server
    participant Runtime as bridgesdk Runtime
    participant API as External API

    Note over Client,API: Inbound webhook handling
    Client->>Server: POST webhook (signed/auth headers)
    Server->>Server: Verify signature / auth
    Server->>Server: Parse payload → map to envelope
    Server->>Runtime: HostAPI().IngestBridgeMessage(envelope)
    Runtime-->>Server: Ack / may request delivery

    Note over Server,API: Outbound delivery
    Server->>Server: Resolve instance config & delivery state
    Server->>Server: Enforce ordering / decide create/edit/delete
    Server->>API: REST call (create/edit/delete)
    API-->>Server: Response (remote id / error / retry-after)
    Server->>Server: Persist delivery state & write markers
    Server->>Runtime: ReportDeliveryResult

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding bridge adapter implementations for Discord, Google Chat, and GitHub.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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

[coderabbitai]

Actionable comments posted: 12

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@extensions/bridges/discord/provider.go`:
- Around line 875-894: The http.Server created as httpServer (used with
p.serveWebhookHTTP and Serve(ln)) has zero timeouts and can be pinned by slow
clients; set at least ReadHeaderTimeout and IdleTimeout on that http.Server
(e.g., ReadHeaderTimeout = 10*time.Second, IdleTimeout = 2*time.Minute) when
constructing it so connections can't be held indefinitely, keeping the rest of
the flow (p.server, p.serverAddr, Serve goroutine) unchanged.
- Around line 752-754: The code reads cfg.APIBaseURL into apiBaseURL (via
firstNonEmpty + normalizeURL) allowing tenant-controlled API hosts which can be
used for token exfiltration/SSRF; change this so provider_config.api_base_url is
not accepted from tenant-managed config: either only allow env/test overrides
(remove cfg.APIBaseURL from firstNonEmpty call) or validate the resolved host
against a strict allowlist of Discord hosts before using it (e.g., check the
hostname of the URL returned by normalizeURL and reject/ignore values not in the
allowlist), ensuring apiBaseURL is only set when the source is an allowed
env/test value.
- Around line 2056-2063: The isNotInitializedRPCError function should not
perform a string-based fallback; remove the strings.Contains() path and rely
only on errors.As() to detect RPC errors by their Code() value: in
isNotInitializedRPCError, keep the interface{ Code() int } extraction via
errors.As(err, &rpcErr) and return rpcErr.Code() == rpcCodeNotInitialized, and
otherwise return false so only typed RPC errors with the matching code are
considered.
- Around line 1022-1024: The handler currently ignores the incoming ctx and uses
context.Background() for inbound processing and async work; replace those with
the provided ctx when calling dispatchInboundEnvelope and when launching
goroutines so request cancellation propagates. In dispatchAsyncInboundEnvelope,
stop deriving contexts from context.Background(); instead derive them from a
provider-owned cancellable root context (e.g., p.ctx/p.cancel or add one if
missing) that is canceled in stop(), and ensure the goroutine selects on both
p.stopCh (or p.ctx.Done()) and ctx.Done() before/while calling
dispatchInboundEnvelope so in-flight work is cancelable during shutdown.

In `@extensions/bridges/gchat/provider.go`:
- Around line 1711-1827: The verification currently calls fetchGoogleX509Keys on
every request (used by verifyDirectBearerToken and verifyPubSubBearerToken) and
blocks on http.DefaultClient with no timeout; change fetchGoogleX509Keys to use
an in-memory cache keyed by certsURL with an expiry/TTL and a singleflight or
mutex to coalesce concurrent refreshes, have verification read cached keys and
only trigger a refresh when expired (or background refresh), and use a bounded
http.Client with an explicit timeout/context when performing the remote GET;
ensure errors when refreshing fall back to existing cached keys (if present) to
avoid making webhook handling dependent on the cert endpoint latency.
- Around line 826-850: The code currently accepts operator-influential URLs from
provider config (apiBaseURL, tokenURL, directCertsURL, pubsubCertsURL) when
building resolvedInstanceConfig; change this to prevent untrusted
provider_config overrides by only using operator-controlled defaults or
validated values: update the logic that sets apiBaseURL, tokenURL,
directCertsURL, and pubsubCertsURL (where firstNonEmpty(...) and
normalizeURL(...) are used) to ignore cfg values unless a flagged operator/test
mode is active, or validate them against a strict allowlist via a new helper
(e.g., isAllowedURL(url string) bool) and only accept the value if it passes
that check; ensure the resolvedInstanceConfig construction uses the
safe/validated URLs and add unit tests for both rejection and acceptance of
allowed URLs.
- Around line 972-989: The http.Server created in this block (httpServer) must
set timeouts to avoid hanging external requests: when constructing httpServer
(used with p.serveWebhookHTTP and Serve(ln)), add ReadHeaderTimeout and
IdleTimeout (and optionally ReadTimeout/WriteTimeout) with sensible durations
(e.g., ReadHeaderTimeout: 5s, IdleTimeout: 2m) instead of zero values; keep
using ln, actualAddr, and the same serve goroutine and error handling but ensure
the server struct includes these timeout fields before assigning p.server and
starting Serve.
- Around line 1067-1088: Several calls to dispatchInboundEnvelope are using
context.Background(), detaching work from request cancellation and shutdown;
replace those context.Background() calls with the real request or provider
context (e.g., use the incoming HTTP request's Context() — r.Context() — in the
webhook handler, or the provider shutdown context (p.ctx) where appropriate) so
dispatchInboundEnvelope(ctx, cfg.instanceID, item.Envelope) uses a cancellable
context; update all four call sites that currently pass context.Background()
(the branches that handle mapped direct messages and other webhook paths,
including the branch that runs when cfg.batcher is nil) to accept/forward the
correct ctx. Ensure any helper functions invoked by the handler also take and
pass the same ctx so cancellation/shutdown propagates.

In `@extensions/bridges/github/provider.go`:
- Around line 212-219: The apiFactory currently returns a new githubClient for
every call which prevents reuse of the in-memory installation-token cache;
change the implementation so each bridge instance reuses a single githubClient
(or at least a shared token cache) per resolvedInstanceConfig instead of
recreating it on every auth check/delivery. Concretely, create and store a
githubClient keyed by resolvedInstanceConfig (or attach it to the provider
instance) and have provider.apiFactory return that stored client; keep using the
same githubClient struct, http.Client with Timeout, and now: func() time.Time {
return provider.now() } when constructing the cached client.
- Around line 1014-1018: storeDeliveryState currently always inserts into
p.deliveries and never removes terminal entries, causing unbounded growth;
update githubProvider.storeDeliveryState to compute key :=
deliveryStateKey(instanceID, deliveryID), then if the incoming state is a
terminal state (implement or use an isTerminalDeliveryState helper) remove
p.deliveries[key] under p.mu (or skip insertion) instead of storing it,
otherwise store/overwrite as before; ensure p.mu locking remains around the
delete/insert to avoid races.
- Around line 587-604: The code currently checks duplicate repos via seenRepos
but does not reject shared webhook paths, which lets a payload signed by
instance A spoof instance B; add a seenWebhookPaths map and in the same
reconciliation loop (where resolveInstanceConfig returns cfg) reject duplicate
webhook paths by setting cfg.configError (similar to the repo conflict) when
cfg.webhookPath != "" and seenWebhookPaths[cfg.webhookPath] exists, e.g.
cfg.configError = fmt.Errorf("github: webhook path %q is already owned by %q and
cannot also belong to %q", cfg.webhookPath, ownerInstance, cfg.instanceID); also
record seenWebhookPaths[cfg.webhookPath] = cfg.instanceID when non-empty so
configsForPath cannot contain multiple instances for the same path (or
alternatively ensure signature verification consults configsForPath and binds
verification to payload.Repository.FullName before accepting).
- Around line 258-264: handleInitialize currently returns before afterInitialize
finishes populating p.routes which allows deliveries to proceed (with only a
500ms wait elsewhere) and fail as "unmanaged bridge instance"; fix by making
initialization wait for routes to be ready instead of launching afterInitialize
in a detached goroutine. Replace the async goroutine call around
p.afterInitialize(session) with a synchronous call or add an explicit readiness
signal (e.g., a ready channel or use p.wg/WaitGroup with a ready chan) that is
closed/fulfilled at the end of afterInitialize and have the caller (or the
delivery wait logic) block on that readiness signal instead of a fixed timeout;
apply the same change for the other occurrence mentioned (lines around the
second async block) to ensure p.routes is fully populated before
handleInitialize returns or before deliveries proceed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9e7c3be4-fccd-4821-bf68-d31efc355397

📥 Commits

Reviewing files that changed from the base of the PR and between 1fffcab and 43b8e84.

⛔ Files ignored due to path filters (144)

• .codex/plans/2026-04-15-bridge-secret-resolution-env-refs.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/_meta.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/_tasks.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/MEMORY.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_01.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_02.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_03.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_04.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_05.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_06.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_07.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_08.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_09.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_10.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_11.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_12.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_13.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_14.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_15.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_16.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/memory/task_17.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/issues/BUG-001.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/issues/BUG-002.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/issues/BUG-003.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/issues/BUG-004.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/issues/BUG-005.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/logs/daemon-integration-package-v-after-fixes.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/daemon-integration-package-v.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/daemon-integration-package.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/daemon-live-qa.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/daemon-targeted-create-bridge-timeout.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/install-managed-focused.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/internal-extension-integration-fixed.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/internal-extension-integration-full-after-fixes.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/internal-extension-integration.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/linear-provider-focused.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/make-test-integration-fixed.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/make-test-integration.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/make-verify-final.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/make-verify.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/mock-telegram-api.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/telegram-integration-package-v.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/telegram-integration-package.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/telegram-provider-focused.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/telegram-targeted-resolve-config-timeout.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/telegram-targeted-resolve-config.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/logs/testutil-package.log is excluded by !**/*.log
• .compozy/tasks/bridge-adapters/qa/screenshots/bridges-page.png is excluded by !**/*.png
• .compozy/tasks/bridge-adapters/qa/test-cases/SMOKE-001.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/SMOKE-002.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/SMOKE-003.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/SMOKE-004.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/SMOKE-005.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/SMOKE-006.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/SMOKE-007.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/SMOKE-008.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-001.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-002.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-003.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-004.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-005.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-006.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-007.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-008.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-009.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-010.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-011.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-012.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-013.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-014.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-015.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-016.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-017.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-018.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-019.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-FUNC-020.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-001.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-002.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-003.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-004.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-005.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-006.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-007.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-008.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-009.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-010.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-011.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-INT-012.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-PERF-001.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-PERF-002.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-PERF-003.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-PERF-004.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-PERF-005.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-PERF-006.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-001.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-002.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-003.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-004.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-005.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-006.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-007.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-008.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-009.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-cases/TC-SEC-010.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-plans/bridge-adapters-regression.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/test-plans/bridge-adapters-test-plan.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/qa/verification-report.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_01.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_02.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_03.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_04.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_05.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_06.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_07.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_08.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_09.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_10.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_11.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_12.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_13.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_14.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_15.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_16.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/task_17.md is excluded by !**/*.md
• extensions/bridges/discord/README.md is excluded by !**/*.md
• extensions/bridges/discord/extension.toml is excluded by !**/*.toml
• extensions/bridges/gchat/extension.toml is excluded by !**/*.toml
• extensions/bridges/github/extension.toml is excluded by !**/*.toml
• extensions/bridges/linear/extension.toml is excluded by !**/*.toml
• extensions/bridges/slack/README.md is excluded by !**/*.md
• extensions/bridges/slack/extension.toml is excluded by !**/*.toml
• extensions/bridges/teams/README.md is excluded by !**/*.md
• extensions/bridges/teams/extension.toml is excluded by !**/*.toml
• extensions/bridges/telegram/README.md is excluded by !**/*.md
• extensions/bridges/telegram/extension.toml is excluded by !**/*.toml
• extensions/bridges/whatsapp/README.md is excluded by !**/*.md
• extensions/bridges/whatsapp/extension.toml is excluded by !**/*.toml
• go.sum is excluded by !**/*.sum
• openapi/agh.json is excluded by !**/*.json
• package.json is excluded by !**/*.json
• sdk/examples/telegram-reference/README.md is excluded by !**/*.md
• sdk/examples/telegram-reference/extension.toml is excluded by !**/*.toml
• sdk/typescript/src/generated/contracts.ts is excluded by !**/generated/**
• web/src/generated/agh-openapi.d.ts is excluded by !**/generated/**

📒 Files selected for processing (156)

• extensions/bridges/discord/main.go
• extensions/bridges/discord/markers.go
• extensions/bridges/discord/provider.go
• extensions/bridges/discord/provider_test.go
• extensions/bridges/gchat/main.go
• extensions/bridges/gchat/markers.go
• extensions/bridges/gchat/provider.go
• extensions/bridges/gchat/provider_test.go
• extensions/bridges/github/api.go
• extensions/bridges/github/main.go
• extensions/bridges/github/markers.go
• extensions/bridges/github/provider.go
• extensions/bridges/github/provider_test.go
• extensions/bridges/linear/api.go
• extensions/bridges/linear/main.go
• extensions/bridges/linear/markers.go
• extensions/bridges/linear/provider.go
• extensions/bridges/linear/provider_test.go
• extensions/bridges/linear/runtime_test.go
• extensions/bridges/slack/main.go
• extensions/bridges/slack/markers.go
• extensions/bridges/slack/provider.go
• extensions/bridges/slack/provider_test.go
• extensions/bridges/teams/main.go
• extensions/bridges/teams/markers.go
• extensions/bridges/teams/provider.go
• extensions/bridges/teams/provider_test.go
• extensions/bridges/telegram/main.go
• extensions/bridges/telegram/markers.go
• extensions/bridges/telegram/provider.go
• extensions/bridges/telegram/provider_test.go
• extensions/bridges/whatsapp/main.go
• extensions/bridges/whatsapp/markers.go
• extensions/bridges/whatsapp/provider.go
• extensions/bridges/whatsapp/provider_test.go
• go.mod
• internal/api/contract/bridges.go
• internal/api/contract/bridges_integration_test.go
• internal/api/contract/bridges_test.go
• internal/api/core/bridges.go
• internal/api/core/bridges_test.go
• internal/api/core/conversions.go
• internal/api/core/coverage_helpers_test.go
• internal/api/core/errors.go
• internal/api/core/errors_test.go
• internal/api/httpapi/bridges_integration_test.go
• internal/api/httpapi/bridges_test.go
• internal/api/httpapi/httpapi_integration_test.go
• internal/api/spec/spec.go
• internal/api/spec/spec_test.go
• internal/api/udsapi/bridges_integration_test.go
• internal/api/udsapi/bridges_test.go
• internal/api/udsapi/udsapi_integration_test.go
• internal/bridges/delivery_broker.go
• internal/bridges/delivery_broker_test.go
• internal/bridges/delivery_projection_test.go
• internal/bridges/delivery_types.go
• internal/bridges/registry.go
• internal/bridges/registry_test.go
• internal/bridges/types.go
• internal/bridges/types_test.go
• internal/bridgesdk/batching.go
• internal/bridgesdk/batching_test.go
• internal/bridgesdk/cache.go
• internal/bridgesdk/cache_test.go
• internal/bridgesdk/dedup.go
• internal/bridgesdk/dedup_test.go
• internal/bridgesdk/doc.go
• internal/bridgesdk/errors.go
• internal/bridgesdk/errors_test.go
• internal/bridgesdk/hostapi.go
• internal/bridgesdk/hostapi_test.go
• internal/bridgesdk/peer.go
• internal/bridgesdk/peer_test.go
• internal/bridgesdk/runtime.go
• internal/bridgesdk/runtime_flow_test.go
• internal/bridgesdk/runtime_integration_test.go
• internal/bridgesdk/runtime_test.go
• internal/bridgesdk/test_helpers_test.go
• internal/bridgesdk/webhook.go
• internal/bridgesdk/webhook_test.go
• internal/cli/bridge.go
• internal/cli/bridge_test.go
• internal/daemon/bridge_secrets.go
• internal/daemon/bridges.go
• internal/daemon/bridges_test.go
• internal/daemon/daemon.go
• internal/daemon/daemon_integration_test.go
• internal/daemon/daemon_test.go
• internal/extension/bridge_delivery_integration_test.go
• internal/extension/bridge_delivery_notifier_test.go
• internal/extension/capability.go
• internal/extension/capability_test.go
• internal/extension/contract/host_api.go
• internal/extension/contract/sdk.go
• internal/extension/discord_provider_integration_test.go
• internal/extension/gchat_provider_integration_test.go
• internal/extension/github_provider_integration_test.go
• internal/extension/host_api.go
• internal/extension/host_api_bridges.go
• internal/extension/host_api_integration_test.go
• internal/extension/host_api_test.go
• internal/extension/install_managed.go
• internal/extension/install_managed_test.go
• internal/extension/linear_provider_integration_test.go
• internal/extension/manager.go
• internal/extension/manager_integration_test.go
• internal/extension/manager_test.go
• internal/extension/manifest.go
• internal/extension/manifest_integration_test.go
• internal/extension/manifest_test.go
• internal/extension/protocol/host_api.go
• internal/extension/protocol/host_api_test.go
• internal/extension/provider_conformance_matrix_integration_test.go
• internal/extension/registry_test.go
• internal/extension/slack_provider_integration_test.go
• internal/extension/teams_provider_integration_test.go
• internal/extension/telegram_provider_integration_test.go
• internal/extension/telegram_reference_integration_test.go
• internal/extension/whatsapp_provider_integration_test.go
• internal/extensiontest/bridge_adapter_harness.go
• internal/extensiontest/bridge_adapter_harness_integration_test.go
• internal/extensiontest/bridge_adapter_harness_test.go
• internal/extensiontest/bridge_conformance_matrix.go
• internal/extensiontest/bridge_conformance_matrix_test.go
• internal/observe/bridges_test.go
• internal/store/globaldb/global_db.go
• internal/store/globaldb/global_db_bridge.go
• internal/store/globaldb/global_db_bridges_integration_test.go
• internal/store/globaldb/global_db_bridges_test.go
• internal/store/globaldb/global_db_extra_test.go
• internal/store/globaldb/migrate_workspace.go
• internal/subprocess/handshake.go
• internal/subprocess/handshake_test.go
• internal/testutil/testutil.go
• internal/testutil/testutil_test.go
• scripts/postinstall.sh
• sdk/examples/telegram-reference/main.go
• sdk/examples/telegram-reference/main_test.go
• sdk/typescript/src/extension.test.ts
• sdk/typescript/src/host-api.test.ts
• sdk/typescript/src/host-api.ts
• web/src/routes/_app/-bridges.test.tsx
• web/src/routes/_app/bridges.tsx
• web/src/systems/bridges/adapters/bridges-api.test.ts
• web/src/systems/bridges/components/bridge-create-dialog.test.tsx
• web/src/systems/bridges/components/bridge-create-dialog.tsx
• web/src/systems/bridges/components/bridge-detail-panel.test.tsx
• web/src/systems/bridges/components/bridge-detail-panel.tsx
• web/src/systems/bridges/hooks/use-bridge-actions.test.tsx
• web/src/systems/bridges/index.ts
• web/src/systems/bridges/lib/bridge-drafts.test.ts
• web/src/systems/bridges/lib/bridge-drafts.ts
• web/src/systems/bridges/lib/bridge-formatters.test.ts
• web/src/systems/bridges/lib/bridge-formatters.ts
• web/src/systems/bridges/types.ts

[coderabbitai]

Actionable comments posted: 5

♻️ Duplicate comments (1)

extensions/bridges/discord/provider.go (1)

968-969: ⚠️ Potential issue | 🟠 Major

The async interaction path is still detached from shutdown cancellation.

dispatchAsyncInboundEnvelope still builds its own context.Background() timeout and only checks p.stopCh once before the host call. Once the goroutine enters dispatchInboundEnvelope, shutdown cannot cancel that work, so it can outlive provider stop. This is the same cancellation gap raised earlier, now limited to the interaction path.

As per coding guidelines, "Every goroutine must have explicit ownership and shutdown via context.Context cancellation".

Also applies to: 977-978, 1056-1072

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/bridges/discord/provider.go` around lines 968 - 969, The async
interaction path spawns goroutines that use context.Background() and only check
p.stopCh once, so shutdown cannot cancel ongoing work; update
dispatchAsyncInboundEnvelope and any callers (the code that checks
cfg.dedup.Mark and calls p.dispatchAsyncInboundEnvelope) to accept and propagate
a cancellable context derived from the provider's stop/context (instead of
creating context.Background()), start goroutines with that context, and ensure
dispatchInboundEnvelope and its internal host calls respect that context (remove
or replace any local context.Background() timeouts with ctx-aware timeouts and
add select checks on ctx.Done() where p.stopCh was previously checked) so every
goroutine is owned and cancels on provider shutdown.

🧹 Nitpick comments (1)

extensions/bridges/discord/provider_test.go (1)

1540-1590: Move these helpers behind t.Helper()/shared testutil instead of unsafe field mutation.

bridgepkgToWebhookRequest hides a marshal failure, and injectedDiscordSession/setUnexportedField couple the test to private bridgesdk.Session layout via reflect+unsafe. A shared helper that takes *testing.T, calls t.Helper(), and owns this setup would be much safer.

As per coding guidelines, "Use t.Helper() on test helper functions" and "Use shared test helpers from internal/testutil and internal/api/testutil".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/bridges/discord/provider_test.go` around lines 1540 - 1590, The
helpers in this test (bridgepkgToWebhookRequest, injectedDiscordSession,
setUnexportedField) are unsafe and hide errors; replace them with proper test
helpers that call t.Helper(), return errors when json.Marshal fails, and avoid
reflect+unsafe by using shared test utilities or exported setup functions: (1)
move bridgepkgToWebhookRequest into internal testutil and change its signature
to accept *testing.T and return (bridgesdk.WebhookRequest) while calling
t.Helper() and failing the test on Marshal error; (2) remove
injectedDiscordSession/setUnexportedField and instead add a testutil constructor
that accepts a HostAPIClient and InstanceCache and initializes a
bridgesdk.Session via the public API or a sanctioned test-only constructor in
internal/api/testutil (also call t.Helper()); (3) update tests to import and use
these new testutil helpers so no reflection/unsafe field mutation remains.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@extensions/bridges/discord/provider_test.go`:
- Around line 641-645: The test currently uses time.Sleep after calling
provider.handleInitialize, which is flaky; instead, wait deterministically for
initialization to finish by replacing the sleep with a synchronization primitive
(e.g., wait on a ready/initialized channel or a sync.WaitGroup) exposed by the
provider or returned by handleInitialize; if no such signal exists, change
provider.handleInitialize (and/or the provider struct) to signal completion (eg.
close an initDone channel or decrement a WaitGroup) and have the test block on
that signal before proceeding or asserting.
- Around line 30-1488: Convert the long, one-off tests into table-driven
subtests using t.Run("Should...") for each scenario: e.g., refactor
TestMapDiscordMessageEventRoutingAndAttachments,
TestMapDiscordInteractionPayloadsStableTargetIdentity,
TestMapDiscordReactionPayloads,
TestExecuteDiscordDeliveryValidatesEditAndDeleteOperations and similar Test*
functions into a table (slice of cases) where each case has a name like "Should
map DM with attachment" and runs as t.Run("Should ...", func(t *testing.T) {
t.Parallel(); setup reused shared test helpers (e.g.,
testDiscordManagedInstance, discordAPIFake, provider setup), execute the
scenario using the case inputs, and assert expected outputs }); keep existing
helper functions and unique identifiers (mapDiscordMessageEvent,
mapDiscordInteractionCommand, mapDiscordInteractionAction,
mapDiscordReactionEvent, executeDiscordDelivery, handleInteractionWebhook,
serveWebhookHTTP, determineInitialState, etc.) to locate logic to test and reuse
setup across cases. Ensure every new subtest uses the "Should..." naming pattern
and preserves t.Parallel() semantics and existing assertions.

In `@extensions/bridges/discord/provider.go`:
- Around line 1040-1049: Reaction events (handled via mapDiscordReactionEvent
and the MESSAGE_REACTION_ADD/REMOVE flow) are missing the DM policy check
present for MESSAGE_CREATE; before deduping (dedup.Mark) and dispatching
(dispatchInboundEnvelope) you must invoke the same allowDiscordDirectMessage
check used for messages to block reactions from disallowed DM users. Update the
reaction handling path to call allowDiscordDirectMessage with the
mapped/envelope context (the same inputs used for MESSAGE_CREATE), return an
appropriate HTTP error or no-content response when disallowed, and only proceed
to dedup.Mark and p.dispatchInboundEnvelope(mapped.Envelope) if the DM policy
allows the event.

In `@extensions/bridges/gchat/provider.go`:
- Around line 1835-1840: Add a check that the Pub/Sub JWT's email is verified:
after the existing issuerMatches and email equality checks (in the same
validation path using issuerMatches, claims.Email and
cfg.pubsubServiceAccountEmail), verify that claims.EmailVerified is true (or
non-nil and true if using a pointer) and return a descriptive error like "gchat:
pubsub bearer email %q is not verified" if not; ensure you normalize/trimsame
strings as you already do for the email comparison and place the check after
signature/audience verification so unverified email claims are rejected.
- Around line 670-678: In reportReadyIfNeeded, don't ignore the error returned
by reportState; capture its error and handle it instead of using the blank
identifier. Call reportState from reportReadyIfNeeded and if it returns an
error, log the failure (using the provider/session logger), do not update
reportedStatus to BridgeStatusReady, and return early so the instance doesn't
become stale; only set reportedStatus[p.trimmed bridgeInstanceID] =
bridgepkg.BridgeStatusReady (under p.mu) after a successful reportState call.
Ensure you remove the "_ = " usage and handle the error explicitly in
reportReadyIfNeeded.

---

Duplicate comments:
In `@extensions/bridges/discord/provider.go`:
- Around line 968-969: The async interaction path spawns goroutines that use
context.Background() and only check p.stopCh once, so shutdown cannot cancel
ongoing work; update dispatchAsyncInboundEnvelope and any callers (the code that
checks cfg.dedup.Mark and calls p.dispatchAsyncInboundEnvelope) to accept and
propagate a cancellable context derived from the provider's stop/context
(instead of creating context.Background()), start goroutines with that context,
and ensure dispatchInboundEnvelope and its internal host calls respect that
context (remove or replace any local context.Background() timeouts with
ctx-aware timeouts and add select checks on ctx.Done() where p.stopCh was
previously checked) so every goroutine is owned and cancels on provider
shutdown.

---

Nitpick comments:
In `@extensions/bridges/discord/provider_test.go`:
- Around line 1540-1590: The helpers in this test (bridgepkgToWebhookRequest,
injectedDiscordSession, setUnexportedField) are unsafe and hide errors; replace
them with proper test helpers that call t.Helper(), return errors when
json.Marshal fails, and avoid reflect+unsafe by using shared test utilities or
exported setup functions: (1) move bridgepkgToWebhookRequest into internal
testutil and change its signature to accept *testing.T and return
(bridgesdk.WebhookRequest) while calling t.Helper() and failing the test on
Marshal error; (2) remove injectedDiscordSession/setUnexportedField and instead
add a testutil constructor that accepts a HostAPIClient and InstanceCache and
initializes a bridgesdk.Session via the public API or a sanctioned test-only
constructor in internal/api/testutil (also call t.Helper()); (3) update tests to
import and use these new testutil helpers so no reflection/unsafe field mutation
remains.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 05e148b4-3ec6-46d0-ac1c-e5ce477b01ec

📥 Commits

Reviewing files that changed from the base of the PR and between 43b8e84 and 0f5a85e.

⛔ Files ignored due to path filters (13)

• .compozy/tasks/bridge-adapters/reviews-001/_meta.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_001.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_002.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_003.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_004.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_005.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_006.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_007.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_008.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_009.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_010.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_011.md is excluded by !**/*.md
• .compozy/tasks/bridge-adapters/reviews-001/issue_012.md is excluded by !**/*.md

📒 Files selected for processing (6)

• extensions/bridges/discord/provider.go
• extensions/bridges/discord/provider_test.go
• extensions/bridges/gchat/provider.go
• extensions/bridges/gchat/provider_test.go
• extensions/bridges/github/provider.go
• extensions/bridges/github/provider_test.go

✅ Files skipped from review due to trivial changes (1)

• extensions/bridges/gchat/provider_test.go

[coderabbitai]

🧹 Nitpick comments (7)

extensions/bridges/gchat/provider.go (6)

298-304: Add compile-time interface verification for gchatBotClient.

The coding guidelines require compile-time interface verification. Add a static assertion to ensure gchatBotClient implements gchatAPI.

As per coding guidelines: "Use compile-time interface verification: var _ Interface = (*Type)(nil)"

♻️ Proposed fix

Add after the gchatAPI interface definition:

var _ gchatAPI = (*gchatBotClient)(nil)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/bridges/gchat/provider.go` around lines 298 - 304, Add a
compile-time interface assertion that ensures gchatBotClient implements
gchatAPI: insert var _ gchatAPI = (*gchatBotClient)(nil) immediately after the
gchatAPI interface definition so the compiler verifies the implementation at
build time; reference the gchatAPI interface and the gchatBotClient concrete
type when adding this statement.

---

1890-1909: Consider releasing lock during remote fetch.

The mutex is held during the entire remote HTTP fetch operation (line 1898). Under high concurrent load, this could cause request pile-up while waiting for a single cert refresh. Consider using singleflight or a read-then-upgrade pattern to allow concurrent cache reads while coalescing refresh operations.

This is a minor optimization concern—the current implementation is correct and the caching TTL minimizes the impact.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/bridges/gchat/provider.go` around lines 1890 - 1909, The cache
lookup currently holds c.mu across the network fetch in get/refresh path; change
to a read-then-upgrade pattern (or use singleflight) so the lock isn't held
during c.fetchRemote: first acquire c.mu and check c.entries[trimmedURL] and
expiry (using c.now()) and return if valid, otherwise release c.mu, perform
keys, expiresAt, err := c.fetchRemote(ctx, trimmedURL) without the lock, then
re-acquire c.mu and re-check c.entries[trimmedURL] (double-check) in case
another goroutine refreshed it; if still expired or missing, store the new
googleX509KeyCacheEntry{keys: keys, expiresAt: expiresAt} into
c.entries[trimmedURL]; ensure error handling follows the same double-check: on
fetch error re-acquire lock and return existing entry.keys if present and
non-empty, else propagate err.

---

701-704: Consider requiring non-nil context.

The function accepts a nil context and falls back to context.Background(). Go convention is that callers should always provide a non-nil context. If this defensive check is intentional for specific call sites, consider documenting why.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/bridges/gchat/provider.go` around lines 701 - 704, The
retryHostCall method on gchatProvider currently silently substitutes a nil
context with context.Background(); change it to require a non-nil context:
remove the fallback to context.Background(), add a guard that returns a clear
error if ctx == nil (or document the requirement in the function comment and let
it panic if you prefer), and update any callers of retryHostCall to pass an
explicit context; reference the retryHostCall method on type gchatProvider when
making these edits so the nil-context handling is enforced and the behavior is
documented.

---

2248-2310: Consider using singleflight for token acquisition.

Two concurrent calls could both perform the full token exchange if they both find the cache expired. This is safe (the work is idempotent) but could be optimized with golang.org/x/sync/singleflight to coalesce concurrent requests for the same token.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/bridges/gchat/provider.go` around lines 2248 - 2310, The
accessToken method can be optimized to coalesce concurrent token fetches using
golang.org/x/sync/singleflight: add a singleflight.Group field (e.g.,
tokenGroup) to gchatBotClient, keep the existing short c.mu-guarded fast-path
that returns cachedToken if valid, and when that misses call
c.tokenGroup.Do("token", func() (interface{}, error) { ... }) to run the JWT
creation + HTTP exchange exactly once for concurrent callers; inside the Do
closure re-check the cache under c.mu before performing the network work, update
c.cachedToken and c.tokenExpiry there, and return the token string (or error) so
callers receive the result without duplicated requests.

---

569-571: Add justification for ignored server shutdown error.

The error from server.Shutdown is discarded. During shutdown, continuing despite an error is often the right choice, but the coding guidelines require either handling the error or documenting why it's safe to ignore.

As per coding guidelines: "Never ignore errors with _ — every error must be handled or have a written justification"

♻️ Proposed fix

 if server != nil {
-    _ = server.Shutdown(shutdownCtx)
+    // Shutdown error is intentionally not propagated: we're already shutting down
+    // and need to proceed with cleanup regardless of whether the HTTP server
+    // stopped gracefully.
+    _ = server.Shutdown(shutdownCtx)
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/bridges/gchat/provider.go` around lines 569 - 571, The error
returned by server.Shutdown(shutdownCtx) is currently discarded; capture its
return value and handle it rather than using `_`. Locate the shutdown code
around the server variable and replace the ignored error with proper handling:
assign err := server.Shutdown(shutdownCtx) and either log it (using the
package/logger in this file) at warning/error level with context like "gchat
server shutdown failed" or, if you truly intend to ignore it, add a clear
comment above the call explaining why ignoring the error is safe per coding
guidelines (including any assumptions about subsequent process termination or
earlier cleanup) and still capture the error in a variable to satisfy linters.

---

946-967: Consider passing a cancellable context to the batcher.

The batcher is constructed with context.Background() which won't propagate provider shutdown signals. While batcher.Close() is called during shutdown, passing a context derived from the provider's lifecycle would provide more graceful cancellation.

♻️ Proposed improvement

Consider creating a provider-scoped context that's cancelled on shutdown:

 if cfg.Batching.DelayMS > 0 {
+    batcherCtx, batcherCancel := context.WithCancel(context.Background())
+    // Store batcherCancel to call during stop()
     batcher, err := bridgesdk.NewInboundBatcher(bridgesdk.InboundBatcherConfig{
-        Context: context.Background(),
+        Context: batcherCtx,

Alternatively, since batcher.Close() is already called in stop(), document why context.Background() is acceptable here.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/bridges/gchat/provider.go` around lines 946 - 967, The batcher is
created with context.Background() which won't cancel on provider shutdown;
change the NewInboundBatcher call to use a provider-scoped context instead
(e.g., use the provider's lifecycle context field such as p.ctx or a context
derived with context.WithCancel that is cancelled in stop()), so that
InboundBatcherConfig.Context is tied to shutdown; update the call sites
referencing bridgesdk.NewInboundBatcher, InboundBatcherConfig, and
resolved.batcher accordingly and ensure existing shutdown logic (e.g.,
batcher.Close() in stop()) still runs.

extensions/bridges/gchat/provider_test.go (1)

696-714: Consider using channel synchronization instead of sleep.

Line 702 uses time.Sleep(20 * time.Millisecond) to simulate delayed route registration. While acceptable in tests, using a channel or other synchronization primitive would be more reliable and faster.

♻️ Alternative approach

 waitDone := make(chan resolvedInstanceConfig, 1)
+routeReady := make(chan struct{})
 go func() {
+    <-routeReady // Wait for route to be set up
     cfg, waitErr := provider.waitForInstanceConfig("brg-gchat", 200*time.Millisecond)
     if waitErr == nil {
         waitDone <- cfg
     }
 }()
-time.Sleep(20 * time.Millisecond)
 provider.mu.Lock()
 provider.routes["brg-gchat"] = resolvedInstanceConfig{instanceID: "brg-gchat"}
 provider.mu.Unlock()
+close(routeReady)

However, this changes the test semantics—the current test specifically validates that waitForInstanceConfig polls and eventually finds the config. The sleep-based approach is valid for this purpose.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@extensions/bridges/gchat/provider_test.go` around lines 696 - 714, Replace
the ad-hoc time.Sleep with explicit channel synchronization: create a signaling
channel (e.g., regDone := make(chan struct{})), start the goroutine that calls
provider.waitForInstanceConfig("brg-gchat", 200*time.Millisecond) and sends to
waitDone as before, then in a separate goroutine set provider.mu.Lock();
provider.routes["brg-gchat"]=resolvedInstanceConfig{instanceID:"brg-gchat"};
provider.mu.Unlock() only after receiving from or waiting on regDone (or
close(regDone)) — remove time.Sleep(20*time.Millisecond) and use regDone to
coordinate the exact moment of route registration so the test deterministically
verifies waitForInstanceConfig finds the config.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@extensions/bridges/gchat/provider_test.go`:
- Around line 696-714: Replace the ad-hoc time.Sleep with explicit channel
synchronization: create a signaling channel (e.g., regDone := make(chan
struct{})), start the goroutine that calls
provider.waitForInstanceConfig("brg-gchat", 200*time.Millisecond) and sends to
waitDone as before, then in a separate goroutine set provider.mu.Lock();
provider.routes["brg-gchat"]=resolvedInstanceConfig{instanceID:"brg-gchat"};
provider.mu.Unlock() only after receiving from or waiting on regDone (or
close(regDone)) — remove time.Sleep(20*time.Millisecond) and use regDone to
coordinate the exact moment of route registration so the test deterministically
verifies waitForInstanceConfig finds the config.

In `@extensions/bridges/gchat/provider.go`:
- Around line 298-304: Add a compile-time interface assertion that ensures
gchatBotClient implements gchatAPI: insert var _ gchatAPI =
(*gchatBotClient)(nil) immediately after the gchatAPI interface definition so
the compiler verifies the implementation at build time; reference the gchatAPI
interface and the gchatBotClient concrete type when adding this statement.
- Around line 1890-1909: The cache lookup currently holds c.mu across the
network fetch in get/refresh path; change to a read-then-upgrade pattern (or use
singleflight) so the lock isn't held during c.fetchRemote: first acquire c.mu
and check c.entries[trimmedURL] and expiry (using c.now()) and return if valid,
otherwise release c.mu, perform keys, expiresAt, err := c.fetchRemote(ctx,
trimmedURL) without the lock, then re-acquire c.mu and re-check
c.entries[trimmedURL] (double-check) in case another goroutine refreshed it; if
still expired or missing, store the new googleX509KeyCacheEntry{keys: keys,
expiresAt: expiresAt} into c.entries[trimmedURL]; ensure error handling follows
the same double-check: on fetch error re-acquire lock and return existing
entry.keys if present and non-empty, else propagate err.
- Around line 701-704: The retryHostCall method on gchatProvider currently
silently substitutes a nil context with context.Background(); change it to
require a non-nil context: remove the fallback to context.Background(), add a
guard that returns a clear error if ctx == nil (or document the requirement in
the function comment and let it panic if you prefer), and update any callers of
retryHostCall to pass an explicit context; reference the retryHostCall method on
type gchatProvider when making these edits so the nil-context handling is
enforced and the behavior is documented.
- Around line 2248-2310: The accessToken method can be optimized to coalesce
concurrent token fetches using golang.org/x/sync/singleflight: add a
singleflight.Group field (e.g., tokenGroup) to gchatBotClient, keep the existing
short c.mu-guarded fast-path that returns cachedToken if valid, and when that
misses call c.tokenGroup.Do("token", func() (interface{}, error) { ... }) to run
the JWT creation + HTTP exchange exactly once for concurrent callers; inside the
Do closure re-check the cache under c.mu before performing the network work,
update c.cachedToken and c.tokenExpiry there, and return the token string (or
error) so callers receive the result without duplicated requests.
- Around line 569-571: The error returned by server.Shutdown(shutdownCtx) is
currently discarded; capture its return value and handle it rather than using
`_`. Locate the shutdown code around the server variable and replace the ignored
error with proper handling: assign err := server.Shutdown(shutdownCtx) and
either log it (using the package/logger in this file) at warning/error level
with context like "gchat server shutdown failed" or, if you truly intend to
ignore it, add a clear comment above the call explaining why ignoring the error
is safe per coding guidelines (including any assumptions about subsequent
process termination or earlier cleanup) and still capture the error in a
variable to satisfy linters.
- Around line 946-967: The batcher is created with context.Background() which
won't cancel on provider shutdown; change the NewInboundBatcher call to use a
provider-scoped context instead (e.g., use the provider's lifecycle context
field such as p.ctx or a context derived with context.WithCancel that is
cancelled in stop()), so that InboundBatcherConfig.Context is tied to shutdown;
update the call sites referencing bridgesdk.NewInboundBatcher,
InboundBatcherConfig, and resolved.batcher accordingly and ensure existing
shutdown logic (e.g., batcher.Close() in stop()) still runs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8cea1043-794e-404a-b689-62516b76d45d

📥 Commits

Reviewing files that changed from the base of the PR and between 0f5a85e and 6061f47.

⛔ Files ignored due to path filters (11)

• .codex/plans/2026-04-15-bridge-web-e2e.md is excluded by !**/*.md
• extensions/bridges/discord/extension.toml is excluded by !**/*.toml
• extensions/bridges/gchat/extension.toml is excluded by !**/*.toml
• extensions/bridges/github/extension.toml is excluded by !**/*.toml
• extensions/bridges/linear/extension.toml is excluded by !**/*.toml
• extensions/bridges/slack/extension.toml is excluded by !**/*.toml
• extensions/bridges/teams/extension.toml is excluded by !**/*.toml
• extensions/bridges/telegram/extension.toml is excluded by !**/*.toml
• extensions/bridges/whatsapp/extension.toml is excluded by !**/*.toml
• openapi/agh.json is excluded by !**/*.json
• sdk/examples/telegram-reference/extension.toml is excluded by !**/*.toml

📒 Files selected for processing (14)

• extensions/bridges/discord/provider.go
• extensions/bridges/discord/provider_test.go
• extensions/bridges/gchat/provider.go
• extensions/bridges/gchat/provider_test.go
• internal/api/contract/bridges.go
• internal/api/core/bridges.go
• internal/api/httpapi/handlers_test.go
• internal/api/httpapi/routes.go
• internal/api/httpapi/stream_helpers_test.go
• internal/api/spec/spec.go
• internal/api/spec/spec_test.go
• internal/api/udsapi/handlers_test.go
• internal/api/udsapi/routes.go
• internal/api/udsapi/stream_helpers_test.go

✅ Files skipped from review due to trivial changes (1)

• extensions/bridges/discord/provider.go

🚧 Files skipped from review as they are similar to previous changes (1)

• extensions/bridges/discord/provider_test.go