Walkthrough

Adds a Google Chat OpenClaw channel manifest, template resolution, a tunnel-audience gate hook, and test coverage for registry inclusion, manifest rendering, and hook behavior.

Changes

Google Chat channel manifest

Estimated code review effort: 4 (Complex) | ~45 minutes

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: googlechat-outbound-auth.ts (outbound auth rewrite preload)

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: googlechat-outbound-auth.test.ts: anchor matching, drift throw, idempotency, short-circuit throws when env unset
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Workaround Analysis block at lines 10-55; prior review PRA-7

PRA-2 Resolve/justify — Source-of-truth review needed: googlechat-trusted-proxy-fetch.ts (trusted proxy fetch rewrite preload)

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: googlechat-trusted-proxy-fetch.test.ts: anchor matching (3 sites), drift throw, idempotency, site mapping
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Workaround Analysis block at lines 11-55; prior review PRA-8

PRA-3 Resolve/justify — Source-of-truth review needed: messaging-bridge-provider.ts:268 (argv secret transit)

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: messaging-bridge-provider.test.ts: configureMessagingBridgeRefreshes success/fail-closed tests
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: SECURITY comment at lines 243-274; prior review PRA-11, PRA-12

PRA-11 Resolve/justify — Service account private key passes through argv to provider refresh configure

• Location: src/lib/onboard/messaging-bridge-provider.ts:268
• Category: security
• Problem: OpenShell `provider refresh configure` ingests refresh material only via `--material KEY=VALUE` argv — no stdin, file, or env-ref transport. Private key transits host-local argv (ps/procfs visible).
• Impact: Host-local secret exposure on trusted host that already holds the key; transient (one configure call); key never enters sandbox. Tracked upstream for --secret-material-file/stdin.
• Recommended action: Add TODO comment with upstream OpenShell issue reference for --secret-material-file/stdin transport. Document that key never enters sandbox (key-out-of-sandbox boundary holds).
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: grep -n 'TODO.*secret-material-file' src/lib/onboard/messaging-bridge-provider.ts should show tracking issue reference
• Missing regression test: None needed for current behavior; when upstream adds file/stdin transport, test that argv path is no longer used
• Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -n 'TODO.*secret-material-file' src/lib/onboard/messaging-bridge-provider.ts should show tracking issue reference.
• Evidence: messaging-bridge-provider.ts:243-274 comment documents risk; prior review PRA-11, PRA-12

PRA-12 Resolve/justify — @ts-nocheck on security-critical auth rewrite preload

• Location: src/lib/messaging/channels/googlechat/runtime/googlechat-outbound-auth.ts:1
• Category: security
• Problem: File uses @ts-nocheck because it manipulates Module._extensions and Module.registerHooks with untyped shapes. Type safety gaps in security-critical patch logic.
• Impact: Type errors in patch logic (anchor regex, short-circuit source) would only surface at runtime in gateway boot.
• Recommended action: Migrate to ESM with proper types, or add JSDoc @typedef for Module._extensions and Module.registerHooks shapes. File follow-up to remove @ts-nocheck.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: File should not have @ts-nocheck; TypeScript compile should pass with strict mode
• Missing regression test: TypeScript compile check in CI for this file
• Done when: The risk is fixed or explicitly justified in the PR. Verification: File should not have @ts-nocheck; TypeScript compile should pass with strict mode.
• Evidence: First line of googlechat-outbound-auth.ts is @ts-nocheck; prior review PRA-9

PRA-13 Resolve/justify — @ts-nocheck on security-critical SSRF guard rewrite preload

• Location: src/lib/messaging/channels/googlechat/runtime/googlechat-trusted-proxy-fetch.ts:1
• Category: security
• Problem: File uses @ts-nocheck for same Module hook manipulation reasons as outbound-auth. Type safety gaps in SSRF guard rewrite.
• Impact: Type errors in anchor regexes (ANCHOR_A/B/C) or replacement logic would only surface at runtime.
• Recommended action: Same as outbound-auth: migrate to typed ESM or add JSDoc types for Module hooks. File follow-up to remove @ts-nocheck.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: File should not have @ts-nocheck; TypeScript compile should pass with strict mode
• Missing regression test: TypeScript compile check in CI for this file
• Done when: The risk is fixed or explicitly justified in the PR. Verification: File should not have @ts-nocheck; TypeScript compile should pass with strict mode.
• Evidence: First line of googlechat-trusted-proxy-fetch.ts is @ts-nocheck; prior review PRA-10

PRA-14 Resolve/justify — Synthetic serviceAccountFile sentinel for channel start gate

• Location: src/lib/messaging/channels/googlechat/manifest.ts:115
• Category: security
• Problem: Renders non-existent path `/nonexistent/googlechat-gateway-minted-no-service-account-file` as serviceAccountFile to satisfy OpenClaw start gate (requires non-empty serviceAccount*). File never delivered or read; outbound-auth preload short-circuits before read.
• Impact: Workaround for upstream missing 'configured'/accessToken credential source. If preload fails to load, sentinel path could be read (but secret scan would catch private key leakage).
• Recommended action: Keep workaround analysis block current (lines 143-158). When upstream adds native accessToken auth mode, remove sentinel and gateway-reload-off fragment. Add test that sentinel path never accessed.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check manifest.ts workaround analysis block lines 143-158 for removal condition tracking
• Missing regression test: Test that verifies serviceAccountFile path is never read at runtime (e.g., fs.readFile spy on sentinel path never called)
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check manifest.ts workaround analysis block lines 143-158 for removal condition tracking.
• Evidence: manifest.ts:98-105 sentinel with comment; workaround analysis at lines 115-130; prior review PRA-13

PRA-15 Resolve/justify — Two nodePreloads target same bundle with independent Module._extensions overrides

• Location: src/lib/messaging/channels/googlechat/manifest.ts:200
• Category: architecture
• Problem: Both googlechat-trusted-proxy-fetch and googlechat-outbound-auth preloads wrap Module._extensions['.js'] and Module.registerHooks independently, targeting the same @openclaw/googlechat dist bundle.
• Impact: Duplicate loader wrapping; potential order-dependent behavior; harder to reason about patch composition.
• Recommended action: Document preload order independence. Consider shared loader-hook installer utility to avoid duplicate Module._extensions wrapping.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check if both preloads install hooks in same IIFE or if order matters; look for shared utility in src/lib/messaging/channels/googlechat/runtime/
• Missing regression test: Test that both preloads can be loaded in either order and both patches apply correctly
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check if both preloads install hooks in same IIFE or if order matters; look for shared utility in src/lib/messaging/channels/googlechat/runtime/.
• Evidence: manifest.ts:200-218 two nodePreloads entries; prior review PRA-19

PRA-16 Resolve/justify — Lazy-require cycle: tunnel-runtime.ts duplicates resolveServicePidDir from services.ts

• Location: src/lib/messaging/channels/googlechat/hooks/tunnel-runtime.ts:22
• Category: architecture
• Problem: tunnel-runtime.ts lazy-requires tunnel/services to access resolveServicePidDir and readCloudflaredState, creating import cycle risk and duplicating PID dir logic.
• Impact: Cycle risk if services.ts ever imports tunnel-runtime; duplicated logic; harder to test PID resolution in isolation.
• Recommended action: Extract pure resolveServicePidDir and validateSandboxName to tunnel/pid-dir.ts (shared module) so both services.ts and tunnel-runtime.ts import eagerly without cycle.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: grep -r 'resolveServicePidDir' src/lib/tunnel/ should show single definition in pid-dir.ts
• Missing regression test: tunnel/pid-dir.test.ts covering resolution logic; both services.ts and tunnel-runtime.ts tests import from shared module
• Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -r 'resolveServicePidDir' src/lib/tunnel/ should show single definition in pid-dir.ts.
• Evidence: tunnel-runtime.ts:22-30 lazy-requires services.ts; prior review PRA-17

PRA-17 Resolve/justify — AppPrincipal discovery sentinel collision assumption unverified

• Location: src/lib/messaging/channels/googlechat/template-resolver.ts:20
• Category: correctness
• Problem: All-zeros sentinel `000000000000000000000` assumed to never collide with real Google-assigned add-on principal (~21 digits). No test or doc reference confirming Google assigns non-zero numeric IDs.
• Impact: If Google ever assigns all-zeros principal, discovery log would not surface real value; operator would see 'missing add-on principal binding' instead of the number.
• Recommended action: Add test or doc reference confirming Google add-on principal is 21-digit non-zero numeric ID. If confirmed, note in test comment.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check template-resolver.test.ts for comment referencing Google documentation or test confirming non-zero assignment
• Missing regression test: Test comment or doc link verifying Google principal format; or test that sentinel != any known real principal format
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check template-resolver.test.ts for comment referencing Google documentation or test confirming non-zero assignment.
• Evidence: template-resolver.ts:19-22 sentinel with comment; test uses 21-digit value but no source reference; prior review PRA-16, PRA-25

PRA-18 Resolve/justify — Gateway reload off fragment is a localized workaround

• Location: src/lib/messaging/channels/googlechat/manifest.ts:98
• Category: architecture
• Problem: Renders gateway.reload.mode=off to prevent OpenClaw's ~60s post-boot self-write from dropping webhook route. Workaround analysis block present (lines 143-158).
• Impact: Disables reactive config reload entirely for sandbox; NemoClaw still restarts gateway explicitly on rebuild/restart. Low risk but upstream fix preferred.
• Recommended action: Keep workaround analysis block current. Remove when upstream reload re-mounts channels (not just plugins) on config reload.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check manifest.ts lines 143-158 for workaround analysis with removal condition
• Missing regression test: Test that gateway reload mode is 'off' in rendered openclaw.json when googlechat channel active
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check manifest.ts lines 143-158 for workaround analysis with removal condition.
• Evidence: manifest.ts:115-130 render fragment + workaround analysis lines 143-158; prior review PRA-18

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-19 Improvement — Per-channel rendered-config-parser boilerplate could be unified

• Location: src/lib/messaging/channels/googlechat/rendered-config-parser.ts:1
• Category: architecture
• Problem: Google Chat has custom RenderedChannelConfigParser (41 lines) that mirrors pattern of other channels but with channel-specific keys.
• Impact: Boilerplate duplication across channels; new channels must copy-paste parser structure.
• Suggested action: Evaluate generic RenderedChannelConfigParser factory that takes manifest and agentId, auto-discovers config inputs with statePath. If not feasible, document why Google Chat needs custom parser.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check if other channels (slack, teams, etc.) have similar parser files with same structure
• Missing regression test: If factory created, test that it produces correct visibility keys for all existing channels
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: rendered-config-parser.ts:41 lines; prior review PRA-20

PRA-20 Improvement — No integration flow test for bridge provider lifecycle with fake OpenShell gateway

• Location: src/lib/onboard/messaging-bridge-provider.test.ts:1
• Category: tests
• Problem: Unit tests mock runOpenshell; no test exercises full bridge provider lifecycle (profile import → provider create → refresh configure) against a fake OpenShell gateway.
• Impact: Integration gaps between bridge wiring and gateway behavior undetected; argv secret transit, refresh timing, provider detachment not validated end-to-end.
• Suggested action: Add integration test using fake OpenShell gateway (or in-process gateway mock) that verifies: profile imported, provider created with sentinel, refresh configured with secret material, token minted and rotated.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Look for test file using fake gateway or in-process openshell mock for bridge provider flow
• Missing regression test: Integration test: googlechat-bridge-provider-lifecycle-with-fake-openshell-gateway
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: messaging-bridge-provider.test.ts only unit tests with mocked runOpenshell; prior review PRA-T6, PRA-T21, PRA-T27

PRA-21 Improvement — Missing health-check hook entirely for Google Chat

• Location: src/lib/messaging/channels/googlechat/manifest.ts:1
• Category: tests
• Problem: Manifest declares no health-check phase hooks. Other channels (Slack, Teams, WeChat) have openclaw-bridge-health hooks. Google Chat needs health checks for both runtime preloads.
• Impact: No automated verification that channel is healthy after rebuild/start; operator must manually check logs.
• Suggested action: Add health-check hook(s) that verify both preloads are installed and patches applied. Can reuse openclawBridgeHealth helper from hook registry.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check manifest.ts hooks array for phase: 'health-check' entries
• Missing regression test: Health check test verifying both preload sentinels present after channel enable + rebuild
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: manifest.ts hooks array has only enroll hooks; prior review PRA-24

PRA-22 Improvement — Non-interactive workaround path test missing

• Location: src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate.ts:70
• Category: tests
• Problem: Unit test covers non-interactive throw behavior but no test documents the workaround rationale or verifies the skip message matches onboarding docs.
• Impact: Behavior is tested but not linked to documented operator guidance; future changes could diverge from docs.
• Suggested action: Add test comment or separate doc test linking the non-interactive skip to onboarding documentation once PRA-6 doc update lands.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check tunnel-audience-gate.test.ts for comment referencing onboarding doc section
• Missing regression test: Doc-linked test or documentation test verifying skip message matches onboarding guide
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: tunnel-audience-gate.test.ts:55-66 tests throw; prior review PRA-26

PRA-23 Improvement — DEFAULT_WEBHOOK_PATH constant used only once

• Location: src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate.ts:15
• Category: architecture
• Problem: Constant `const DEFAULT_WEBHOOK_PATH = "/googlechat";` defined at line 15, used only at line 130. Manifest render also hardcodes webhookPath: "/googlechat".
• Impact: Drift risk between constant and manifest hardcode; unnecessary abstraction adds cognitive load without reuse benefit.
• Suggested action: Inline the constant or derive from manifest's webhookPath render value to avoid drift. If kept as constant, ensure single source of truth.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: grep -n 'DEFAULT_WEBHOOK_PATH' src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate.ts should show 1 definition + 1 usage, or 0 if inlined
• Missing regression test: None needed for simplification
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: simplificationSignals shows single_use_config at line 15 and 130

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

• PRA-23 delete (src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate.ts:15): const DEFAULT_WEBHOOK_PATH = "/googlechat"; at line 15
  • Replacement: Use literal "/googlechat" at line 130, or import from manifest's webhookPath render value
  • Net: -1 lines
  • Safety boundary: webhookPath in manifest render (line 103) must remain "/googlechat" — this is the contract with plugin inbound route

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — googlechat-boot-health-check-verifies-outbound-auth-patch-applied. Runtime/sandbox/infrastructure paths need behavioral runtime validation: boot-time health checks for preloads, bridge provider integration flow with fake gateway, full channel enable→rebuild→send/receive cycle.
• PRA-T2 Runtime validation — googlechat-boot-health-check-verifies-trusted-proxy-fetch-patch-applied. Runtime/sandbox/infrastructure paths need behavioral runtime validation: boot-time health checks for preloads, bridge provider integration flow with fake gateway, full channel enable→rebuild→send/receive cycle.
• PRA-T3 Runtime validation — googlechat-bridge-provider-lifecycle-with-fake-openshell-gateway. Runtime/sandbox/infrastructure paths need behavioral runtime validation: boot-time health checks for preloads, bridge provider integration flow with fake gateway, full channel enable→rebuild→send/receive cycle.
• PRA-T4 Runtime validation — googlechat-non-interactive-skip-documented-in-onboarding-guide. Runtime/sandbox/infrastructure paths need behavioral runtime validation: boot-time health checks for preloads, bridge provider integration flow with fake gateway, full channel enable→rebuild→send/receive cycle.
• PRA-T5 Runtime validation — googlechat-appPrincipal-sentinel-collision-assumption-verified-vs-google-docs. Runtime/sandbox/infrastructure paths need behavioral runtime validation: boot-time health checks for preloads, bridge provider integration flow with fake gateway, full channel enable→rebuild→send/receive cycle.
• PRA-T6 No integration flow test for bridge provider lifecycle with fake OpenShell gateway — Add integration test using fake OpenShell gateway (or in-process gateway mock) that verifies: profile imported, provider created with sentinel, refresh configured with secret material, token minted and rotated.
• PRA-T7 Missing health-check hook entirely for Google Chat — Add health-check hook(s) that verify both preloads are installed and patches applied. Can reuse openclawBridgeHealth helper from hook registry.
• PRA-T8 Non-interactive workaround path test missing — Add test comment or separate doc test linking the non-interactive skip to onboarding documentation once PRA-6 doc update lands.

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: googlechat-outbound-auth.ts (outbound auth rewrite preload)

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: googlechat-outbound-auth.test.ts: anchor matching, drift throw, idempotency, short-circuit throws when env unset
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Workaround Analysis block at lines 10-55; prior review PRA-7

PRA-2 Resolve/justify — Source-of-truth review needed: googlechat-trusted-proxy-fetch.ts (trusted proxy fetch rewrite preload)

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: googlechat-trusted-proxy-fetch.test.ts: anchor matching (3 sites), drift throw, idempotency, site mapping
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Workaround Analysis block at lines 11-55; prior review PRA-8

PRA-3 Resolve/justify — Source-of-truth review needed: messaging-bridge-provider.ts:268 (argv secret transit)

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: messaging-bridge-provider.test.ts: configureMessagingBridgeRefreshes success/fail-closed tests
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: SECURITY comment at lines 243-274; prior review PRA-11, PRA-12

PRA-4 Required — 455-line monolith for generic bridge provider wiring

• Location: src/lib/onboard/messaging-bridge-provider.ts:1
• Category: correctness
• Problem: Single file handles profile discovery, token def collection, profile import, refresh configuration, and secret resolution mixed together. Exceeds reviewable size and mixes multiple responsibilities.
• Impact: Hard to review security boundaries; changes to one concern risk breaking another; no clear module boundaries for testing or future channels.
• Required action: Extract into focused modules: list-bridge-profiles.ts, collect-bridge-token-defs.ts, ensure-bridge-profiles.ts, configure-bridge-refreshes.ts, bridge-secret-resolution.ts, plus a thin barrel export. Each <150 lines.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: grep -r 'export.*MessagingBridge' src/lib/onboard/ --include="*.ts" | wc -l should show multiple focused files after extraction
• Missing regression test: Each extracted module should have its own test file mirroring current messaging-bridge-provider.test.ts coverage
• Done when: The required change is committed and verification passes: grep -r 'export.*MessagingBridge' src/lib/onboard/ --include="*.ts" | wc -l should show multiple focused files after extraction.
• Evidence: File is 455 lines with 5 exported functions handling distinct phases; prior review PRA-2

PRA-5 Required — Test monolith grew to 1041 lines with Google Chat tests

• Location: src/lib/messaging/channels/manifests.test.ts:1
• Category: correctness
• Problem: manifests.test.ts now contains all channel manifest tests including 163 lines of Google Chat specific tests.
• Impact: Test file too large for effective review; changes to one channel's tests risk breaking others; slows test iteration.
• Required action: Split into per-channel test files (e.g., googlechat.manifest.test.ts, teams.manifest.test.ts) or by concern (hooks, render, runtime, packages). Target <500 lines per file.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: wc -l src/lib/messaging/channels/*.test.ts should show no file >500 lines
• Missing regression test: Each split test file must preserve all current test coverage for its channel
• Done when: The required change is committed and verification passes: wc -l src/lib/messaging/channels/*.test.ts should show no file >500 lines.
• Evidence: File grew from 918 to 1041 lines (+123) with Google Chat additions; prior review PRA-3

PRA-6 Required — providers.ts monolith grew to 556 lines with bridge orchestration

• Location: src/lib/onboard/providers.ts:1
• Category: correctness
• Problem: providers.ts mixes inference provider selection, hosted inference staging, gateway provider CRUD, and now bridge provider orchestration via upsertMessagingProviders.
• Impact: Bridge provider logic coupled to inference provider logic; changes to one domain risk the other; exceeds single-responsibility.
• Required action: Extract bridge provider orchestration into messaging-bridge-upsert.ts that upsertMessagingProviders delegates to. Keep providers.ts focused on inference/provider selection.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: grep -n 'upsertMessagingProviders' src/lib/onboard/providers.ts should show only a thin delegation call after extraction
• Missing regression test: New messaging-bridge-upsert.test.ts covering the delegation and error paths
• Done when: The required change is committed and verification passes: grep -n 'upsertMessagingProviders' src/lib/onboard/providers.ts should show only a thin delegation call after extraction.
• Evidence: File grew from 507 to 556 lines (+49) with bridge wiring; prior review PRA-4

PRA-7 Required — tunnel/services.ts monolith grew to 771 lines with lazy-require cycle

• Location: src/lib/tunnel/services.ts:640
• Category: correctness
• Problem: services.ts handles cloudflared lifecycle, gateway stop, PID dir resolution. tunnel-runtime.ts lazy-requires services.ts creating import cycle risk; resolveServicePidDir duplicated.
• Impact: Import cycle risk; duplicated PID dir resolution; hard to test PID logic in isolation.
• Required action: Extract PID directory resolution into tunnel/pid-dir.ts used by both services.ts and tunnel-runtime.ts. Consider splitting services.ts into cloudflared-lifecycle.ts, gateway-stop.ts, pid-dir.ts.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: grep -r 'resolveServicePidDir' src/lib/tunnel/ should show single definition in pid-dir.ts imported by both
• Missing regression test: tunnel/pid-dir.test.ts covering resolution logic with various env/sandbox combinations
• Done when: The required change is committed and verification passes: grep -r 'resolveServicePidDir' src/lib/tunnel/ should show single definition in pid-dir.ts imported by both.
• Evidence: File grew from 749 to 771 lines (+22); tunnel-runtime.ts:22-30 duplicates resolveServicePidDir via lazy require; prior review PRA-5, PRA-17

PRA-8 Required — Non-interactive mode unconditionally skips Google Chat even with pre-supplied audience

• Location: src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate.ts:70
• Category: correctness
• Problem: Hook throws at line 50-62 when context.isInteractive===false, even if GOOGLECHAT_AUDIENCE is pre-configured. Documented as intentional (Cloud Console + appPrincipal steps need human) but not in onboarding docs.
• Impact: Automation/CI cannot enable Google Chat non-interactively even with all config pre-supplied; operators surprised by skip.
• Required action: Document in onboarding docs: Google Chat requires interactive enrollment (Cloud Console endpoint URL + appPrincipal). If automation needed later, hook must be updated to support pre-captured appPrincipal + audience.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Check docs/onboarding.md or similar for 'Google Chat' and 'non-interactive' sections
• Missing regression test: Doc update verified; or if automation path added later, test that pre-supplied audience + appPrincipal bypasses skip
• Done when: The required change is committed and verification passes: Check docs/onboarding.md or similar for 'Google Chat' and 'non-interactive' sections.
• Evidence: tunnel-audience-gate.ts:50-62 throws unconditionally; test at tunnel-audience-gate.test.ts:55-66 confirms behavior; prior review PRA-6

PRA-9 Required — Missing boot-time health check for googlechat-outbound-auth patch

• Location: src/lib/messaging/channels/googlechat/runtime/googlechat-outbound-auth.ts:60
• Category: security
• Problem: Patch drift logs but continues gateway boot; fail-closed only at send time. Channel can start with unpatched plugin and fail only on first outbound reply.
• Impact: Silent degradation: bot receives messages but cannot reply until first send attempt; operator unaware until user reports broken replies.
• Required action: Add boot-time health check hook for googlechat that verifies process.__nemoclawGooglechatOutboundAuthInstalled and patch applied. Fail gateway startup if patch missing and channel active.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Check manifest.ts hooks array for a health-check phase hook that validates the outbound-auth sentinel
• Missing regression test: Integration test: start gateway with googlechat channel active but outbound-auth preload disabled; verify gateway fails to start
• Done when: The required change is committed and verification passes: Check manifest.ts hooks array for a health-check phase hook that validates the outbound-auth sentinel.
• Evidence: googlechat-outbound-auth.ts:225-236 catches drift, logs, continues; prior review PRA-14, PRA-T7, PRA-22

PRA-10 Required — Missing boot-time health check for googlechat-trusted-proxy-fetch patch

• Location: src/lib/messaging/channels/googlechat/runtime/googlechat-trusted-proxy-fetch.ts:85
• Category: security
• Problem: Patch drift logs but continues gateway boot; fail-closed only at fetch time (cert verify or outbound send). Channel can start with unpatched plugin and fail on first inbound cert verify or outbound call.
• Impact: Silent degradation: bot starts but inbound verification fails (401 on webhook) or outbound fails; operator unaware until first message.
• Required action: Add boot-time health check hook verifying process.__nemoclawGooglechatTrustedProxyFetchInstalled and all three anchors patched. Fail gateway startup if patch missing and channel active.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Check manifest.ts hooks array for a health-check phase hook that validates the trusted-proxy-fetch sentinel
• Missing regression test: Integration test: start gateway with googlechat channel active but trusted-proxy-fetch preload disabled; verify gateway fails to start
• Done when: The required change is committed and verification passes: Check manifest.ts hooks array for a health-check phase hook that validates the trusted-proxy-fetch sentinel.
• Evidence: googlechat-trusted-proxy-fetch.ts:240-250 catches drift, logs, continues; prior review PRA-15, PRA-T8, PRA-23

PRA-11 Resolve/justify — Service account private key passes through argv to provider refresh configure

• Location: src/lib/onboard/messaging-bridge-provider.ts:268
• Category: security
• Problem: OpenShell `provider refresh configure` ingests refresh material only via `--material KEY=VALUE` argv — no stdin, file, or env-ref transport. Private key transits host-local argv (ps/procfs visible).
• Impact: Host-local secret exposure on trusted host that already holds the key; transient (one configure call); key never enters sandbox. Tracked upstream for --secret-material-file/stdin.
• Recommended action: Add TODO comment with upstream OpenShell issue reference for --secret-material-file/stdin transport. Document that key never enters sandbox (key-out-of-sandbox boundary holds).
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: grep -n 'TODO.*secret-material-file' src/lib/onboard/messaging-bridge-provider.ts should show tracking issue reference
• Missing regression test: None needed for current behavior; when upstream adds file/stdin transport, test that argv path is no longer used
• Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -n 'TODO.*secret-material-file' src/lib/onboard/messaging-bridge-provider.ts should show tracking issue reference.
• Evidence: messaging-bridge-provider.ts:243-274 comment documents risk; prior review PRA-11, PRA-12

PRA-12 Resolve/justify — @ts-nocheck on security-critical auth rewrite preload

• Location: src/lib/messaging/channels/googlechat/runtime/googlechat-outbound-auth.ts:1
• Category: security
• Problem: File uses @ts-nocheck because it manipulates Module._extensions and Module.registerHooks with untyped shapes. Type safety gaps in security-critical patch logic.
• Impact: Type errors in patch logic (anchor regex, short-circuit source) would only surface at runtime in gateway boot.
• Recommended action: Migrate to ESM with proper types, or add JSDoc @typedef for Module._extensions and Module.registerHooks shapes. File follow-up to remove @ts-nocheck.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: File should not have @ts-nocheck; TypeScript compile should pass with strict mode
• Missing regression test: TypeScript compile check in CI for this file
• Done when: The risk is fixed or explicitly justified in the PR. Verification: File should not have @ts-nocheck; TypeScript compile should pass with strict mode.
• Evidence: First line of googlechat-outbound-auth.ts is @ts-nocheck; prior review PRA-9

PRA-13 Resolve/justify — @ts-nocheck on security-critical SSRF guard rewrite preload

• Location: src/lib/messaging/channels/googlechat/runtime/googlechat-trusted-proxy-fetch.ts:1
• Category: security
• Problem: File uses @ts-nocheck for same Module hook manipulation reasons as outbound-auth. Type safety gaps in SSRF guard rewrite.
• Impact: Type errors in anchor regexes (ANCHOR_A/B/C) or replacement logic would only surface at runtime.
• Recommended action: Same as outbound-auth: migrate to typed ESM or add JSDoc types for Module hooks. File follow-up to remove @ts-nocheck.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: File should not have @ts-nocheck; TypeScript compile should pass with strict mode
• Missing regression test: TypeScript compile check in CI for this file
• Done when: The risk is fixed or explicitly justified in the PR. Verification: File should not have @ts-nocheck; TypeScript compile should pass with strict mode.
• Evidence: First line of googlechat-trusted-proxy-fetch.ts is @ts-nocheck; prior review PRA-10

PRA-14 Resolve/justify — Synthetic serviceAccountFile sentinel for channel start gate

• Location: src/lib/messaging/channels/googlechat/manifest.ts:115
• Category: security
• Problem: Renders non-existent path `/nonexistent/googlechat-gateway-minted-no-service-account-file` as serviceAccountFile to satisfy OpenClaw start gate (requires non-empty serviceAccount*). File never delivered or read; outbound-auth preload short-circuits before read.
• Impact: Workaround for upstream missing 'configured'/accessToken credential source. If preload fails to load, sentinel path could be read (but secret scan would catch private key leakage).
• Recommended action: Keep workaround analysis block current (lines 143-158). When upstream adds native accessToken auth mode, remove sentinel and gateway-reload-off fragment. Add test that sentinel path never accessed.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check manifest.ts workaround analysis block lines 143-158 for removal condition tracking
• Missing regression test: Test that verifies serviceAccountFile path is never read at runtime (e.g., fs.readFile spy on sentinel path never called)
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check manifest.ts workaround analysis block lines 143-158 for removal condition tracking.
• Evidence: manifest.ts:98-105 sentinel with comment; workaround analysis at lines 115-130; prior review PRA-13

PRA-15 Resolve/justify — Two nodePreloads target same bundle with independent Module._extensions overrides

• Location: src/lib/messaging/channels/googlechat/manifest.ts:200
• Category: architecture
• Problem: Both googlechat-trusted-proxy-fetch and googlechat-outbound-auth preloads wrap Module._extensions['.js'] and Module.registerHooks independently, targeting the same @openclaw/googlechat dist bundle.
• Impact: Duplicate loader wrapping; potential order-dependent behavior; harder to reason about patch composition.
• Recommended action: Document preload order independence. Consider shared loader-hook installer utility to avoid duplicate Module._extensions wrapping.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check if both preloads install hooks in same IIFE or if order matters; look for shared utility in src/lib/messaging/channels/googlechat/runtime/
• Missing regression test: Test that both preloads can be loaded in either order and both patches apply correctly
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check if both preloads install hooks in same IIFE or if order matters; look for shared utility in src/lib/messaging/channels/googlechat/runtime/.
• Evidence: manifest.ts:200-218 two nodePreloads entries; prior review PRA-19

PRA-16 Resolve/justify — Lazy-require cycle: tunnel-runtime.ts duplicates resolveServicePidDir from services.ts

• Location: src/lib/messaging/channels/googlechat/hooks/tunnel-runtime.ts:22
• Category: architecture
• Problem: tunnel-runtime.ts lazy-requires tunnel/services to access resolveServicePidDir and readCloudflaredState, creating import cycle risk and duplicating PID dir logic.
• Impact: Cycle risk if services.ts ever imports tunnel-runtime; duplicated logic; harder to test PID resolution in isolation.
• Recommended action: Extract pure resolveServicePidDir and validateSandboxName to tunnel/pid-dir.ts (shared module) so both services.ts and tunnel-runtime.ts import eagerly without cycle.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: grep -r 'resolveServicePidDir' src/lib/tunnel/ should show single definition in pid-dir.ts
• Missing regression test: tunnel/pid-dir.test.ts covering resolution logic; both services.ts and tunnel-runtime.ts tests import from shared module
• Done when: The risk is fixed or explicitly justified in the PR. Verification: grep -r 'resolveServicePidDir' src/lib/tunnel/ should show single definition in pid-dir.ts.
• Evidence: tunnel-runtime.ts:22-30 lazy-requires services.ts; prior review PRA-17

PRA-17 Resolve/justify — AppPrincipal discovery sentinel collision assumption unverified

• Location: src/lib/messaging/channels/googlechat/template-resolver.ts:20
• Category: correctness
• Problem: All-zeros sentinel `000000000000000000000` assumed to never collide with real Google-assigned add-on principal (~21 digits). No test or doc reference confirming Google assigns non-zero numeric IDs.
• Impact: If Google ever assigns all-zeros principal, discovery log would not surface real value; operator would see 'missing add-on principal binding' instead of the number.
• Recommended action: Add test or doc reference confirming Google add-on principal is 21-digit non-zero numeric ID. If confirmed, note in test comment.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check template-resolver.test.ts for comment referencing Google documentation or test confirming non-zero assignment
• Missing regression test: Test comment or doc link verifying Google principal format; or test that sentinel != any known real principal format
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check template-resolver.test.ts for comment referencing Google documentation or test confirming non-zero assignment.
• Evidence: template-resolver.ts:19-22 sentinel with comment; test uses 21-digit value but no source reference; prior review PRA-16, PRA-25

PRA-18 Resolve/justify — Gateway reload off fragment is a localized workaround

• Location: src/lib/messaging/channels/googlechat/manifest.ts:98
• Category: architecture
• Problem: Renders gateway.reload.mode=off to prevent OpenClaw's ~60s post-boot self-write from dropping webhook route. Workaround analysis block present (lines 143-158).
• Impact: Disables reactive config reload entirely for sandbox; NemoClaw still restarts gateway explicitly on rebuild/restart. Low risk but upstream fix preferred.
• Recommended action: Keep workaround analysis block current. Remove when upstream reload re-mounts channels (not just plugins) on config reload.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check manifest.ts lines 143-158 for workaround analysis with removal condition
• Missing regression test: Test that gateway reload mode is 'off' in rendered openclaw.json when googlechat channel active
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check manifest.ts lines 143-158 for workaround analysis with removal condition.
• Evidence: manifest.ts:115-130 render fragment + workaround analysis lines 143-158; prior review PRA-18

PRA-19 Improvement — Per-channel rendered-config-parser boilerplate could be unified

• Location: src/lib/messaging/channels/googlechat/rendered-config-parser.ts:1
• Category: architecture
• Problem: Google Chat has custom RenderedChannelConfigParser (41 lines) that mirrors pattern of other channels but with channel-specific keys.
• Impact: Boilerplate duplication across channels; new channels must copy-paste parser structure.
• Suggested action: Evaluate generic RenderedChannelConfigParser factory that takes manifest and agentId, auto-discovers config inputs with statePath. If not feasible, document why Google Chat needs custom parser.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check if other channels (slack, teams, etc.) have similar parser files with same structure
• Missing regression test: If factory created, test that it produces correct visibility keys for all existing channels
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: rendered-config-parser.ts:41 lines; prior review PRA-20

PRA-20 Improvement — No integration flow test for bridge provider lifecycle with fake OpenShell gateway

• Location: src/lib/onboard/messaging-bridge-provider.test.ts:1
• Category: tests
• Problem: Unit tests mock runOpenshell; no test exercises full bridge provider lifecycle (profile import → provider create → refresh configure) against a fake OpenShell gateway.
• Impact: Integration gaps between bridge wiring and gateway behavior undetected; argv secret transit, refresh timing, provider detachment not validated end-to-end.
• Suggested action: Add integration test using fake OpenShell gateway (or in-process gateway mock) that verifies: profile imported, provider created with sentinel, refresh configured with secret material, token minted and rotated.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Look for test file using fake gateway or in-process openshell mock for bridge provider flow
• Missing regression test: Integration test: googlechat-bridge-provider-lifecycle-with-fake-openshell-gateway
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: messaging-bridge-provider.test.ts only unit tests with mocked runOpenshell; prior review PRA-T6, PRA-T21, PRA-T27

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: Google Chat bridge refresh material transport

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Covered by the blocker's requested `googlechat bridge refresh configures private_key without argv exposure` test.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the blocker finding `Google service-account private key is passed through OpenShell argv`.

PRA-2 Resolve/justify — Source-of-truth review needed: Google Chat trusted-proxy fetch preload

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Pure helper tests cover representative anchors, but the requested runtime validation should prove the installed extension bundle is patched and URL confinement still holds.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Active Google Chat runtime monkeypatch drift is logged instead of failing closed`.

PRA-3 Resolve/justify — Source-of-truth review needed: Google Chat outbound-auth source rewrite

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Pure helper tests cover the source rewrite and placeholder behavior, but active installed-bundle validation is missing.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Active Google Chat runtime monkeypatch drift is logged instead of failing closed`.

PRA-4 Resolve/justify — Source-of-truth review needed: Rendered Google Chat `serviceAccountFile` sentinel

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Requested runtime test `googlechat start uses the gateway-minted sentinel without reading serviceAccountFile`.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Google Chat sentinels and reload workaround lack source-boundary runtime validation`.

PRA-5 Resolve/justify — Source-of-truth review needed: Google Chat appPrincipal discovery sentinel

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Requested runtime test `googlechat appPrincipal discovery sentinel remains non-authorizing and logs the real principal`.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Google Chat sentinels and reload workaround lack source-boundary runtime validation`.

PRA-6 Resolve/justify — Source-of-truth review needed: Google Chat `gateway.reload.mode=off` workaround

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Requested runtime test `googlechat route survives post-boot config self-write with gateway reload disabled`.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Google Chat sentinels and reload workaround lack source-boundary runtime validation`.

PRA-9 Resolve/justify — Google Chat service-account JSON and audience values are under-validated

• Location: src/lib/onboard/messaging-bridge-provider.ts:323
• Category: security
• Problem: The bridge material path only checks that the pasted JSON parses and contains non-empty `client_email` and `private_key` strings. It does not bound the JSON size, validate a plausible service-account email, require PEM delimiters or reasonable private-key length, or validate Google Chat audience values before they are rendered into OpenClaw config. The DM allowlist prompt says emails are ignored, but the resolver passes email-like entries through unchanged.
• Impact: Malformed or oversized secrets can reach OpenShell refresh configuration and fail late; invalid audiences can render a channel that cannot verify inbound JWTs; and permissive allowlist shaping can drift from the documented authorization model if upstream starts accepting identifiers differently.
• Recommended action: Reject empty and oversized service-account JSON; require a plausible service-account `client_email`; require PEM delimiters and reasonable private-key length bounds; validate `app-url` audiences as HTTPS URLs ending in `/googlechat`; validate `project-number` audiences as plausible numeric project numbers; and either reject non-`users/<id>` allowlist entries or document/test the exact upstream behavior. Keep all error messages redacted.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Read `buildRefreshMaterial()` in `src/lib/onboard/messaging-bridge-provider.ts`, the Google Chat manifest input definitions, and `resolveGooglechatTemplateReference()`; confirm invalid service-account JSON, invalid PEM, non-HTTPS audiences, wrong webhook paths, and email allowlist entries are rejected or intentionally normalized before rendering or refresh configuration.
• Missing regression test: Add `googlechat bridge rejects malformed service-account material before openshell argv`, `googlechat audience validation rejects non-HTTPS app-url and non-numeric project-number`, and `googlechat DM allowlist rejects email entries when matching is ID-only` covering the validation boundary.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `buildRefreshMaterial()` in `src/lib/onboard/messaging-bridge-provider.ts`, the Google Chat manifest input definitions, and `resolveGooglechatTemplateReference()`; confirm invalid service-account JSON, invalid PEM, non-HTTPS audiences, wrong webhook paths, and email allowlist entries are rejected or intentionally normalized before rendering or refresh configuration.
• Evidence: `buildRefreshMaterial()` catches JSON parse errors and checks only truthiness for `client_email` and `private_key`; `GOOGLECHAT_AUDIENCE` has no `formatPattern`; `template-resolver.test.ts` currently expects `users/111, [email protected]` to render as both entries.

PRA-10 Resolve/justify — Active Google Chat runtime monkeypatch drift is logged instead of failing closed

• Location: src/lib/messaging/channels/googlechat/runtime/googlechat-outbound-auth.ts:234
• Category: security
• Problem: The outbound-auth and trusted-proxy-fetch preloads are security-boundary workarounds: one prevents in-sandbox service-account signing, and the other changes Google API fetches to trusted proxy modes. Their pure helpers throw named drift errors, but the preload installers catch those errors, write a diagnostic, and allow gateway boot to continue. The manifest declares no active health-check hook that proves the installed extension bundle was patched.
• Impact: If the pinned plugin bundle shape drifts or loader interception fails, an enabled Google Chat sandbox can start with the intended security behavior absent. Depending on which patch fails, outbound auth may fall back toward in-sandbox signing/broken credential paths or Google API fetches may run outside the intended DNS-less trusted-proxy assumptions until the first runtime failure.
• Recommended action: Add active Google Chat start/health validation that locates the installed package or extension bundle, confirms both patch anchors are present, confirms patched behavior is active, and fails closed for enabled Google Chat on drift. Keep tolerant no-crash behavior only for inactive or explicitly legacy configurations, and distinguish install/read failures from anchor drift in diagnostics.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect `googlechat-outbound-auth.ts`, `googlechat-trusted-proxy-fetch.ts`, and `googlechatManifest.runtime.openclaw`; confirm that an active Google Chat plan has a health/start validation path that fails when either patch is not applied to the installed extension bundle, not only unit tests against representative source slices.
• Missing regression test: Add `active googlechat health fails when outbound-auth or trusted-proxy anchors drift in the installed extension bundle`; mock a sandbox bundle missing each anchor and assert the health/start path aborts the active channel with a clear diagnostic.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect `googlechat-outbound-auth.ts`, `googlechat-trusted-proxy-fetch.ts`, and `googlechatManifest.runtime.openclaw`; confirm that an active Google Chat plan has a health/start validation path that fails when either patch is not applied to the installed extension bundle, not only unit tests against representative source slices.
• Evidence: Both runtime files catch patch installation errors near the bottom and write `patch NOT applied` messages; `googlechatManifest` lists `nodePreloads` and `secretScans` but no `health-check` hook for Google Chat; current tests exercise `patchSource()` on representative snippets rather than an active installed bundle.

PRA-11 Resolve/justify — Broad `www.googleapis.com` cert egress remains unconstrained and untested

• Location: src/lib/messaging/channels/googlechat/policy/openclaw.yaml:38
• Category: security
• Problem: The Google Chat preset allows GET `/**` on `www.googleapis.com` for inbound JWT verification certs. The comment explains path variability, but there are no focused policy tests proving the intended cert-fetch surface or denying unrelated Google APIs on the same host.
• Impact: The sandbox gains a broad read egress surface to `www.googleapis.com`; if sandbox code or the plugin can issue arbitrary GETs to that host, the policy may permit unrelated Google API reads beyond the Google Chat cert use case.
• Recommended action: Prefer concrete certificate/JWK paths if the policy language and Google auth library behavior allow it. If path variability makes that impractical, add focused policy tests that assert the intended Chat API and cert-fetch surfaces plus a negative test for unrelated `www.googleapis.com` paths; if the evaluator cannot express denial, document that evaluator limitation beside the rule.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Read `src/lib/messaging/channels/googlechat/policy/openclaw.yaml` and `test/policies.test.ts`; confirm there is a positive assertion for the Google Chat cert path and a negative assertion for an unrelated `www.googleapis.com` path, or an explicit documented evaluator limitation.
• Missing regression test: Add `googlechat policy denies unrelated www.googleapis.com paths while allowing Google cert fetches`; include positive Chat `POST/PATCH/DELETE /v1/spaces/**` and cert GET cases, and a negative unrelated `www.googleapis.com` path.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/messaging/channels/googlechat/policy/openclaw.yaml` and `test/policies.test.ts`; confirm there is a positive assertion for the Google Chat cert path and a negative assertion for an unrelated `www.googleapis.com` path, or an explicit documented evaluator limitation.
• Evidence: The policy contains `host: www.googleapis.com\` with `rules: - allow: { method: GET, path: "/**" }`; grep/read of `test/policies.test.ts` showed Google Chat only in preset-name inclusion, not path-level policy coverage.

PRA-12 Resolve/justify — Google Chat sentinels and reload workaround lack source-boundary runtime validation

• Location: src/lib/messaging/channels/googlechat/manifest.ts:129
• Category: architecture
• Problem: The manifest renders three localized workarounds: a synthetic `serviceAccountFile` start-gate marker, an all-zero `appPrincipal` discovery sentinel, and `gateway.reload.mode=off`. Comments now document invalid states and intended exits, but the tests only assert static render/template shape and do not prove the source-boundary behavior at runtime.
• Impact: If upstream OpenClaw begins reading `serviceAccountFile`, treats the all-zero principal as authorizing, changes the principal mismatch log, or changes gateway reload behavior, NemoClaw can silently ship a channel that either leaks credential assumptions, fails inbound authorization, or loses the webhook route after boot.
• Recommended action: Add focused runtime/start validation proving the gateway-minted path starts without reading the sentinel file, the appPrincipal sentinel remains non-authorizing while still surfacing the real principal, and the Google Chat route survives OpenClaw's post-boot config self-write with `gateway.reload=off`. Prefer upstream/source fixes that make these invalid states impossible when available.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Read `googlechatManifest.render`, `resolveGooglechatTemplateReference()`, and the changed tests; confirm coverage goes beyond static JSON assertions and exercises the OpenClaw start/gateway behavior that makes each sentinel safe.
• Missing regression test: Add `googlechat start uses the gateway-minted sentinel without reading serviceAccountFile`, `googlechat appPrincipal discovery sentinel remains non-authorizing and logs the real principal`, and `googlechat route survives post-boot config self-write with gateway reload disabled`.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `googlechatManifest.render`, `resolveGooglechatTemplateReference()`, and the changed tests; confirm coverage goes beyond static JSON assertions and exercises the OpenClaw start/gateway behavior that makes each sentinel safe.
• Evidence: `manifests.test.ts` asserts strings such as the sentinel path, all-zero appPrincipal is covered only in `template-resolver.test.ts`, and no Google Chat health/runtime hook is declared to validate the route or sentinel behavior after rebuild.

PRA-13 Resolve/justify — The generic bridge-provider convention is incomplete for its only consumer

• Location: src/lib/onboard/messaging-bridge-provider.ts:1
• Category: architecture
• Problem: This PR adds a 455-line generic provider-profile discovery/configuration subsystem for one current channel. The create/onboard path uses it via `messaging-prep.ts` and `providers.ts`, remove uses `bridgeProviderNamesForChannel()`, but direct `channels add` still bypasses it. Malformed provider profiles are also silently skipped because `parseProfileYaml()` returns null.
• Impact: The new abstraction gives a false sense that bridge-backed channels are handled generically while a primary lifecycle path still fails. Silent profile skipping can make a built-in bridge disappear from provider creation without a loud test failure.
• Recommended action: Either finish the generic convention so the same metadata drives onboarding/create, `channels add`, `channels remove`, refresh configuration, rollback, and cleanup, or shrink this to a focused Google Chat bridge path until a second minted-token channel exists. Built-in malformed provider profiles should fail loudly in tests.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Trace `collectMessagingBridgeTokenDefs()` call sites and confirm create/onboard, direct add, remove, refresh configuration, and rollback all use the same provider profile metadata; then corrupt the built-in Google Chat profile in a test fixture and confirm discovery fails loudly rather than returning no profile.
• Missing regression test: Add `malformed built-in googlechat provider-profile fails loudly` and extend the direct add test so bridge profile metadata drives both provider creation and rollback.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Trace `collectMessagingBridgeTokenDefs()` call sites and confirm create/onboard, direct add, remove, refresh configuration, and rollback all use the same provider profile metadata; then corrupt the built-in Google Chat profile in a test fixture and confirm discovery fails loudly rather than returning no profile.
• Evidence: `messaging-prep.ts` calls `collectMessagingBridgeTokenDefs()`, `providers.ts` brackets provider upsert with profile import and refresh configure, `policy-channel.ts` only imports `bridgeProviderNamesForChannel`, and `parseProfileYaml()` returns null on malformed YAML or missing fields.

PRA-14 Resolve/justify — Detailed Google Chat assertions are concentrated in the registry-wide manifest test

• Location: src/lib/messaging/channels/manifests.test.ts:926
• Category: tests
• Problem: The registry-wide `manifests.test.ts` grew with many Google Chat-specific assertions about sentinels, reload behavior, runtime preloads, and plugin integrity. Those are channel-local behavior contracts rather than registry invariants.
• Impact: Centralizing detailed channel behavior in a broad manifest test increases monolith growth and makes future channel-specific workaround changes harder to review. It also obscures which tests should be updated when only Google Chat changes.
• Recommended action: Move detailed Google Chat manifest/runtime/workaround assertions to `src/lib/messaging/channels/googlechat/manifest.test.ts` or similarly local tests. Keep `manifests.test.ts` to registry invariants and a small assertion that Google Chat is registered and available only for OpenClaw.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect `src/lib/messaging/channels/manifests.test.ts` and confirm only cross-channel registry invariants remain there, with Google Chat-specific sentinel/preload/package assertions moved to a Google Chat-local test file.
• Missing regression test: Existing assertions can be preserved by moving them; add a local `googlechat manifest declares gateway-minted auth, sentinels, preloads, and package pins` test and keep a small registry test for built-in registration.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect `src/lib/messaging/channels/manifests.test.ts` and confirm only cross-channel registry invariants remain there, with Google Chat-specific sentinel/preload/package assertions moved to a Google Chat-local test file.
• Evidence: `manifests.test.ts` gained a large `declares Google Chat as an OpenClaw-only inbound-webhook channel` block with channel-specific strings and workaround details.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

• None.

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

• PRA-13 yagni (src/lib/onboard/messaging-bridge-provider.ts:1): The single-consumer generic provider-profile discovery/configuration layer if it is not completed across all channel lifecycles.
  • Replacement: Either complete the convention across direct add/remove/rollback or replace it with a smaller explicit Google Chat bridge helper until another minted-token channel needs the abstraction.
  • Safety boundary: Do not remove the key-out-of-sandbox boundary, credential redaction, provider refresh fail-closed behavior, or bridge provider cleanup.
• PRA-14 shrink (src/lib/messaging/channels/manifests.test.ts:926): Google Chat-specific assertion block from the registry-wide manifest test.
  • Replacement: A Google Chat-local manifest test plus a minimal registry invariant in `manifests.test.ts`.
  • Net: 0 lines
  • Safety boundary: Preserve assertions for credential non-delivery, runtime preloads, package pins, and OpenClaw-only availability; only move their location.

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — channels add googlechat creates and refresh-configures the gateway-minted bridge before persisting the messaging plan. This PR changes sandbox runtime, network policy, credential refresh, package installation, and monkeypatch behavior. The added unit tests are useful, but several claims require behavioral lifecycle or runtime validation rather than representative source-slice tests.
• PRA-T2 Runtime validation — channels add googlechat rolls back preset and refresh-configure failures without persisting the plan. This PR changes sandbox runtime, network policy, credential refresh, package installation, and monkeypatch behavior. The added unit tests are useful, but several claims require behavioral lifecycle or runtime validation rather than representative source-slice tests.
• PRA-T3 Runtime validation — googlechat bridge refresh configures private_key without argv exposure. This PR changes sandbox runtime, network policy, credential refresh, package installation, and monkeypatch behavior. The added unit tests are useful, but several claims require behavioral lifecycle or runtime validation rather than representative source-slice tests.
• PRA-T4 Runtime validation — googlechat bridge rejects malformed service-account material before openshell argv. This PR changes sandbox runtime, network policy, credential refresh, package installation, and monkeypatch behavior. The added unit tests are useful, but several claims require behavioral lifecycle or runtime validation rather than representative source-slice tests.
• PRA-T5 Runtime validation — googlechat audience validation rejects non-HTTPS app-url and non-numeric project-number. This PR changes sandbox runtime, network policy, credential refresh, package installation, and monkeypatch behavior. The added unit tests are useful, but several claims require behavioral lifecycle or runtime validation rather than representative source-slice tests.
• PRA-T6 Detailed Google Chat assertions are concentrated in the registry-wide manifest test — Move detailed Google Chat manifest/runtime/workaround assertions to `src/lib/messaging/channels/googlechat/manifest.test.ts` or similarly local tests. Keep `manifests.test.ts` to registry invariants and a small assertion that Google Chat is registered and available only for OpenClaw.
• PRA-T7 Acceptance clause — Parts of feat(messaging): onboard additional experimental messaging channels #5492 — add test evidence or identify existing coverage. The deterministic context provides no linked issue body or comments for feat(messaging): onboard additional experimental messaging channels #5492, so the literal issue acceptance clauses could not be mapped beyond the PR body's untrusted statement.
• PRA-T8 Acceptance clause — New `googlechat` manifest (inputs, enrollment notes, render) + built-in registration; served on the shared gateway dashboard port at `/googlechat` (no host forward — reuses the tunnel). — add test evidence or identify existing coverage. `googlechatManifest` exists, built-in registration is added, tests assert OpenClaw availability and no `hostForward`, and render includes `webhookPath: "/googlechat"`; direct channel-add still fails to create the required gateway-minted bridge provider.

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: Google Chat bridge refresh material transport

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Covered by the blocker's requested `googlechat bridge refresh configures private_key without argv exposure` test.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the blocker finding `Google service-account private key is passed through OpenShell argv`.

PRA-2 Resolve/justify — Source-of-truth review needed: Google Chat trusted-proxy fetch preload

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Pure helper tests cover representative anchors, but the requested runtime validation should prove the installed extension bundle is patched and URL confinement still holds.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Active Google Chat runtime monkeypatch drift is logged instead of failing closed`.

PRA-3 Resolve/justify — Source-of-truth review needed: Google Chat outbound-auth source rewrite

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Pure helper tests cover the source rewrite and placeholder behavior, but active installed-bundle validation is missing.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Active Google Chat runtime monkeypatch drift is logged instead of failing closed`.

PRA-4 Resolve/justify — Source-of-truth review needed: Rendered Google Chat `serviceAccountFile` sentinel

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Requested runtime test `googlechat start uses the gateway-minted sentinel without reading serviceAccountFile`.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Google Chat sentinels and reload workaround lack source-boundary runtime validation`.

PRA-5 Resolve/justify — Source-of-truth review needed: Google Chat appPrincipal discovery sentinel

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Requested runtime test `googlechat appPrincipal discovery sentinel remains non-authorizing and logs the real principal`.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Google Chat sentinels and reload workaround lack source-boundary runtime validation`.

PRA-6 Resolve/justify — Source-of-truth review needed: Google Chat `gateway.reload.mode=off` workaround

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Requested runtime test `googlechat route survives post-boot config self-write with gateway reload disabled`.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Represented by the finding `Google Chat sentinels and reload workaround lack source-boundary runtime validation`.

PRA-7 Required — `channels add googlechat` still skips the gateway-minted bridge provider

• Location: src/lib/actions/sandbox/policy-channel.ts:995
• Category: correctness
• Problem: The direct channel-add path still builds gateway provider definitions only from `collectManifestCredentials(manifest)`. Google Chat intentionally declares `credentials: []`, so `applyChannelAddToGatewayAndRegistry()` receives an empty `acquired` map, returns `false`, and never sends a `GOOGLE_CHAT_ACCESS_TOKEN` token definition with `providerType: "google-chat-bridge"` through the OpenShell profile import/create/refresh path.
• Impact: A user can persist the Google Chat messaging plan and apply the Google Chat network policy without creating or refresh-configuring the gateway provider that mints outbound Chat API tokens. The sandbox then appears enabled but outbound replies cannot authenticate; preset or refresh failures for this bridge-backed channel are also not rolled back in the direct add lifecycle.
• Required action: Feed bridge-backed channel metadata into `channels add`: build Google Chat's bridge token definition with `collectMessagingBridgeTokenDefs()` or an equivalent manifest/profile-owned API, preserve `providerType: "google-chat-bridge"`, and call the same provider import/create/refresh path before persisting the messaging plan. Include rollback/residual reporting for preset and refresh-configure failures.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Read `src/lib/actions/sandbox/policy-channel.ts` around `addSandboxChannelUnlocked()` and confirm the Google Chat path no longer relies solely on `collectManifestCredentials(manifest)`; the provider upsert should receive `name: "<sandbox>-googlechat-bridge"`, `envKey: "GOOGLE_CHAT_ACCESS_TOKEN"`, `token: MESSAGING_BRIDGE_PENDING_VALUE`, and `providerType: "google-chat-bridge"` before `MessagingHostStateApplier.applyPlanToRegistry()`.
• Missing regression test: Add `channels add googlechat creates and refresh-configures the gateway-minted bridge before persisting the messaging plan` in `test/channels-add-preset.test.ts`; set `GOOGLECHAT_SERVICE_ACCOUNT`, stub the provider path, assert the Google Chat bridge token definition is passed before the registry update, and add companion failures for preset apply and refresh configure rollback/residual reporting.
• Done when: The required change is committed and verification passes: Read `src/lib/actions/sandbox/policy-channel.ts` around `addSandboxChannelUnlocked()` and confirm the Google Chat path no longer relies solely on `collectManifestCredentials(manifest)`; the provider upsert should receive `name: "<sandbox>-googlechat-bridge"`, `envKey: "GOOGLE_CHAT_ACCESS_TOKEN"`, `token: MESSAGING_BRIDGE_PENDING_VALUE`, and `providerType: "google-chat-bridge"` before `MessagingHostStateApplier.applyPlanToRegistry()`.
• Evidence: `policy-channel.ts` imports `bridgeProviderNamesForChannel` but not `collectMessagingBridgeTokenDefs`; `addSandboxChannelUnlocked()` sets `const acquired = collectManifestCredentials(manifest)`; `googlechatManifest.credentials` is `[]`; `applyChannelAddToGatewayAndRegistry()` only upserts when `Object.entries(acquired)` is non-empty.

PRA-8 Required — Google service-account private key is passed through OpenShell argv

• Location: src/lib/onboard/messaging-bridge-provider.ts:397
• Category: security
• Problem: `configureMessagingBridgeRefreshes()` expands refresh material into `--material key=value` arguments, including `private_key=<service-account PEM>`, and passes those argv entries to `openshell provider refresh configure`. The new comment acknowledges the host-local exposure but the transport remains unchanged.
• Impact: The Google service-account private key can be exposed to same-host process inspection surfaces such as `ps` or `/proc/<pid>/cmdline`, and may leak through command auditing or debugging. The key stays out of the sandbox, but the host credential boundary is still weakened for a long-lived signing key.
• Required action: Use a non-argv secret transport before enabling this path: stdin, a 0600 temporary file with cleanup, or an env-reference/secret-handle that OpenShell resolves without echoing the value in argv. If OpenShell cannot ingest refresh material without argv exposure yet, keep Google Chat disabled or skip refresh configuration until that interface exists.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Read `configureMessagingBridgeRefreshes()` and confirm no `private_key` value is concatenated into an argv string or passed as a `--material` argument; verify the OpenShell call carries only a file descriptor, file path with 0600 cleanup, or opaque secret reference.
• Missing regression test: Add `googlechat bridge refresh configures private_key without argv exposure`; stub `runOpenshell`, provide service-account JSON, and assert no argv element contains the PEM or `private_key=<value>` while refresh configuration still receives the secret through the chosen safe transport.
• Done when: The required change is committed and verification passes: Read `configureMessagingBridgeRefreshes()` and confirm no `private_key` value is concatenated into an argv string or passed as a `--material` argument; verify the OpenShell call carries only a file descriptor, file path with 0600 cleanup, or opaque secret reference.
• Evidence: `buildRefreshMaterial()` returns `{ key: "private_key", value: privateKey }`; `materialArgs` is built as `["--material", `${key}=${value}`]`; the resulting array is passed to `deps.runOpenshell(["provider", "refresh", "configure", ...materialArgs, ...])`.

PRA-9 Resolve/justify — Google Chat service-account JSON and audience values are under-validated

• Location: src/lib/onboard/messaging-bridge-provider.ts:323
• Category: security
• Problem: The bridge material path only checks that the pasted JSON parses and contains non-empty `client_email` and `private_key` strings. It does not bound the JSON size, validate a plausible service-account email, require PEM delimiters or reasonable private-key length, or validate Google Chat audience values before they are rendered into OpenClaw config. The DM allowlist prompt says emails are ignored, but the resolver passes email-like entries through unchanged.
• Impact: Malformed or oversized secrets can reach OpenShell refresh configuration and fail late; invalid audiences can render a channel that cannot verify inbound JWTs; and permissive allowlist shaping can drift from the documented authorization model if upstream starts accepting identifiers differently.
• Recommended action: Reject empty and oversized service-account JSON; require a plausible service-account `client_email`; require PEM delimiters and reasonable private-key length bounds; validate `app-url` audiences as HTTPS URLs ending in `/googlechat`; validate `project-number` audiences as plausible numeric project numbers; and either reject non-`users/<id>` allowlist entries or document/test the exact upstream behavior. Keep all error messages redacted.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Read `buildRefreshMaterial()` in `src/lib/onboard/messaging-bridge-provider.ts`, the Google Chat manifest input definitions, and `resolveGooglechatTemplateReference()`; confirm invalid service-account JSON, invalid PEM, non-HTTPS audiences, wrong webhook paths, and email allowlist entries are rejected or intentionally normalized before rendering or refresh configuration.
• Missing regression test: Add `googlechat bridge rejects malformed service-account material before openshell argv`, `googlechat audience validation rejects non-HTTPS app-url and non-numeric project-number`, and `googlechat DM allowlist rejects email entries when matching is ID-only` covering the validation boundary.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `buildRefreshMaterial()` in `src/lib/onboard/messaging-bridge-provider.ts`, the Google Chat manifest input definitions, and `resolveGooglechatTemplateReference()`; confirm invalid service-account JSON, invalid PEM, non-HTTPS audiences, wrong webhook paths, and email allowlist entries are rejected or intentionally normalized before rendering or refresh configuration.
• Evidence: `buildRefreshMaterial()` catches JSON parse errors and checks only truthiness for `client_email` and `private_key`; `GOOGLECHAT_AUDIENCE` has no `formatPattern`; `template-resolver.test.ts` currently expects `users/111, [email protected]` to render as both entries.

PRA-10 Resolve/justify — Active Google Chat runtime monkeypatch drift is logged instead of failing closed

• Location: src/lib/messaging/channels/googlechat/runtime/googlechat-outbound-auth.ts:234
• Category: security
• Problem: The outbound-auth and trusted-proxy-fetch preloads are security-boundary workarounds: one prevents in-sandbox service-account signing, and the other changes Google API fetches to trusted proxy modes. Their pure helpers throw named drift errors, but the preload installers catch those errors, write a diagnostic, and allow gateway boot to continue. The manifest declares no active health-check hook that proves the installed extension bundle was patched.
• Impact: If the pinned plugin bundle shape drifts or loader interception fails, an enabled Google Chat sandbox can start with the intended security behavior absent. Depending on which patch fails, outbound auth may fall back toward in-sandbox signing/broken credential paths or Google API fetches may run outside the intended DNS-less trusted-proxy assumptions until the first runtime failure.
• Recommended action: Add active Google Chat start/health validation that locates the installed package or extension bundle, confirms both patch anchors are present, confirms patched behavior is active, and fails closed for enabled Google Chat on drift. Keep tolerant no-crash behavior only for inactive or explicitly legacy configurations, and distinguish install/read failures from anchor drift in diagnostics.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect `googlechat-outbound-auth.ts`, `googlechat-trusted-proxy-fetch.ts`, and `googlechatManifest.runtime.openclaw`; confirm that an active Google Chat plan has a health/start validation path that fails when either patch is not applied to the installed extension bundle, not only unit tests against representative source slices.
• Missing regression test: Add `active googlechat health fails when outbound-auth or trusted-proxy anchors drift in the installed extension bundle`; mock a sandbox bundle missing each anchor and assert the health/start path aborts the active channel with a clear diagnostic.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect `googlechat-outbound-auth.ts`, `googlechat-trusted-proxy-fetch.ts`, and `googlechatManifest.runtime.openclaw`; confirm that an active Google Chat plan has a health/start validation path that fails when either patch is not applied to the installed extension bundle, not only unit tests against representative source slices.
• Evidence: Both runtime files catch patch installation errors near the bottom and write `patch NOT applied` messages; `googlechatManifest` lists `nodePreloads` and `secretScans` but no `health-check` hook for Google Chat; current tests exercise `patchSource()` on representative snippets rather than an active installed bundle.

PRA-11 Resolve/justify — Broad `www.googleapis.com` cert egress remains unconstrained and untested

• Location: src/lib/messaging/channels/googlechat/policy/openclaw.yaml:38
• Category: security
• Problem: The Google Chat preset allows GET `/**` on `www.googleapis.com` for inbound JWT verification certs. The comment explains path variability, but there are no focused policy tests proving the intended cert-fetch surface or denying unrelated Google APIs on the same host.
• Impact: The sandbox gains a broad read egress surface to `www.googleapis.com`; if sandbox code or the plugin can issue arbitrary GETs to that host, the policy may permit unrelated Google API reads beyond the Google Chat cert use case.
• Recommended action: Prefer concrete certificate/JWK paths if the policy language and Google auth library behavior allow it. If path variability makes that impractical, add focused policy tests that assert the intended Chat API and cert-fetch surfaces plus a negative test for unrelated `www.googleapis.com` paths; if the evaluator cannot express denial, document that evaluator limitation beside the rule.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Read `src/lib/messaging/channels/googlechat/policy/openclaw.yaml` and `test/policies.test.ts`; confirm there is a positive assertion for the Google Chat cert path and a negative assertion for an unrelated `www.googleapis.com` path, or an explicit documented evaluator limitation.
• Missing regression test: Add `googlechat policy denies unrelated www.googleapis.com paths while allowing Google cert fetches`; include positive Chat `POST/PATCH/DELETE /v1/spaces/**` and cert GET cases, and a negative unrelated `www.googleapis.com` path.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/messaging/channels/googlechat/policy/openclaw.yaml` and `test/policies.test.ts`; confirm there is a positive assertion for the Google Chat cert path and a negative assertion for an unrelated `www.googleapis.com` path, or an explicit documented evaluator limitation.
• Evidence: The policy contains `host: www.googleapis.com\` with `rules: - allow: { method: GET, path: "/**" }`; grep/read of `test/policies.test.ts` showed Google Chat only in preset-name inclusion, not path-level policy coverage.

PRA-12 Resolve/justify — Google Chat sentinels and reload workaround lack source-boundary runtime validation

• Location: src/lib/messaging/channels/googlechat/manifest.ts:129
• Category: architecture
• Problem: The manifest renders three localized workarounds: a synthetic `serviceAccountFile` start-gate marker, an all-zero `appPrincipal` discovery sentinel, and `gateway.reload.mode=off`. Comments now document invalid states and intended exits, but the tests only assert static render/template shape and do not prove the source-boundary behavior at runtime.
• Impact: If upstream OpenClaw begins reading `serviceAccountFile`, treats the all-zero principal as authorizing, changes the principal mismatch log, or changes gateway reload behavior, NemoClaw can silently ship a channel that either leaks credential assumptions, fails inbound authorization, or loses the webhook route after boot.
• Recommended action: Add focused runtime/start validation proving the gateway-minted path starts without reading the sentinel file, the appPrincipal sentinel remains non-authorizing while still surfacing the real principal, and the Google Chat route survives OpenClaw's post-boot config self-write with `gateway.reload=off`. Prefer upstream/source fixes that make these invalid states impossible when available.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Read `googlechatManifest.render`, `resolveGooglechatTemplateReference()`, and the changed tests; confirm coverage goes beyond static JSON assertions and exercises the OpenClaw start/gateway behavior that makes each sentinel safe.
• Missing regression test: Add `googlechat start uses the gateway-minted sentinel without reading serviceAccountFile`, `googlechat appPrincipal discovery sentinel remains non-authorizing and logs the real principal`, and `googlechat route survives post-boot config self-write with gateway reload disabled`.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `googlechatManifest.render`, `resolveGooglechatTemplateReference()`, and the changed tests; confirm coverage goes beyond static JSON assertions and exercises the OpenClaw start/gateway behavior that makes each sentinel safe.
• Evidence: `manifests.test.ts` asserts strings such as the sentinel path, all-zero appPrincipal is covered only in `template-resolver.test.ts`, and no Google Chat health/runtime hook is declared to validate the route or sentinel behavior after rebuild.

PRA-13 Resolve/justify — The generic bridge-provider convention is incomplete for its only consumer

• Location: src/lib/onboard/messaging-bridge-provider.ts:1
• Category: architecture
• Problem: This PR adds a 455-line generic provider-profile discovery/configuration subsystem for one current channel. The create/onboard path uses it via `messaging-prep.ts` and `providers.ts`, remove uses `bridgeProviderNamesForChannel()`, but direct `channels add` still bypasses it. Malformed provider profiles are also silently skipped because `parseProfileYaml()` returns null.
• Impact: The new abstraction gives a false sense that bridge-backed channels are handled generically while a primary lifecycle path still fails. Silent profile skipping can make a built-in bridge disappear from provider creation without a loud test failure.
• Recommended action: Either finish the generic convention so the same metadata drives onboarding/create, `channels add`, `channels remove`, refresh configuration, rollback, and cleanup, or shrink this to a focused Google Chat bridge path until a second minted-token channel exists. Built-in malformed provider profiles should fail loudly in tests.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Trace `collectMessagingBridgeTokenDefs()` call sites and confirm create/onboard, direct add, remove, refresh configuration, and rollback all use the same provider profile metadata; then corrupt the built-in Google Chat profile in a test fixture and confirm discovery fails loudly rather than returning no profile.
• Missing regression test: Add `malformed built-in googlechat provider-profile fails loudly` and extend the direct add test so bridge profile metadata drives both provider creation and rollback.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Trace `collectMessagingBridgeTokenDefs()` call sites and confirm create/onboard, direct add, remove, refresh configuration, and rollback all use the same provider profile metadata; then corrupt the built-in Google Chat profile in a test fixture and confirm discovery fails loudly rather than returning no profile.
• Evidence: `messaging-prep.ts` calls `collectMessagingBridgeTokenDefs()`, `providers.ts` brackets provider upsert with profile import and refresh configure, `policy-channel.ts` only imports `bridgeProviderNamesForChannel`, and `parseProfileYaml()` returns null on malformed YAML or missing fields.

PRA-14 Resolve/justify — Detailed Google Chat assertions are concentrated in the registry-wide manifest test

• Location: src/lib/messaging/channels/manifests.test.ts:926
• Category: tests
• Problem: The registry-wide `manifests.test.ts` grew with many Google Chat-specific assertions about sentinels, reload behavior, runtime preloads, and plugin integrity. Those are channel-local behavior contracts rather than registry invariants.
• Impact: Centralizing detailed channel behavior in a broad manifest test increases monolith growth and makes future channel-specific workaround changes harder to review. It also obscures which tests should be updated when only Google Chat changes.
• Recommended action: Move detailed Google Chat manifest/runtime/workaround assertions to `src/lib/messaging/channels/googlechat/manifest.test.ts` or similarly local tests. Keep `manifests.test.ts` to registry invariants and a small assertion that Google Chat is registered and available only for OpenClaw.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect `src/lib/messaging/channels/manifests.test.ts` and confirm only cross-channel registry invariants remain there, with Google Chat-specific sentinel/preload/package assertions moved to a Google Chat-local test file.
• Missing regression test: Existing assertions can be preserved by moving them; add a local `googlechat manifest declares gateway-minted auth, sentinels, preloads, and package pins` test and keep a small registry test for built-in registration.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect `src/lib/messaging/channels/manifests.test.ts` and confirm only cross-channel registry invariants remain there, with Google Chat-specific sentinel/preload/package assertions moved to a Google Chat-local test file.
• Evidence: `manifests.test.ts` gained a large `declares Google Chat as an OpenClaw-only inbound-webhook channel` block with channel-specific strings and workaround details.

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• messaging-providers (high): Closest existing live coverage for messaging provider creation, placeholder isolation, policy application, L7 credential rewriting, and installed OpenClaw channel runtime contracts affected by the new Google Chat provider/profile/runtime wiring.
• channels-add-remove (high): Policy-channel add/remove behavior changed, including bridge registration messaging and provider teardown. Run the live CLI add/rebuild/remove flow to catch registry, gateway provider, and policy lifecycle regressions.
• network-policy (high): The PR adds a security-sensitive Google Chat policy preset and modifies channel policy application paths; run live restricted-sandbox policy add/enforcement coverage.
• credential-sanitization (high): Google Chat introduces service-account secret handling and gateway-minted credentials. Existing live credential-boundary coverage should run to ensure secrets/placeholders are not leaked into sandbox-visible state or artifacts.
• tunnel-lifecycle (high): Google Chat enrollment depends on cloudflared tunnel state/start/URL/stop behavior and this PR touches tunnel service integration; run the real tunnel lifecycle contract.
• cloud-onboard (high): Onboarding provider and messaging prep code changed; run full hosted onboarding to catch regressions in install/onboard/sandbox bootstrap with the built-in messaging manifest registry present.

Optional E2E

• openclaw-plugin-runtime-exdev-e2e (medium): Useful adjacent confidence for OpenClaw plugin installation/runtime dependency handling because the PR adds @openclaw/googlechat to trusted plugin archive/provenance paths, but it is not a direct Google Chat flow.

New E2E recommendations

• googlechat-channel-e2e (high): Existing messaging E2Es cover Telegram/Discord/Slack/WhatsApp patterns but do not exercise Google Chat’s unique gateway-minted service-account JWT provider, provider-profile import, refresh configure, trusted-proxy fetch preload, outbound-auth preload, or removal of a bridge-backed channel with no channelTokenKeys.
  • Suggested test: Add a live or hermetic Google Chat channel add/remove E2E that enables googlechat with a fake/minimal service-account JSON where possible, verifies the google-chat-bridge profile/provider and GOOGLE_CHAT_ACCESS_TOKEN placeholder wiring, asserts the private key is absent from openclaw.json/sandbox files, checks chat.googleapis.com policy presence, rebuilds with the node preloads installed, then removes the channel and verifies the gateway-minted bridge provider is deleted.
• googlechat-tunnel-enrollment-e2e (medium): The Google Chat tunnel/audience gate starts cloudflared, derives the public /googlechat audience, prompts for Cloud Console confirmation, and may stop a self-started tunnel on decline; current tunnel-lifecycle E2E does not drive this channel enrollment hook.
  • Suggested test: Add an interactive or scripted Google Chat enrollment E2E that runs nemoclaw <sandbox> channels add googlechat through the tunnel audience gate and validates derived GOOGLECHAT_AUDIENCE, prompt behavior, and cleanup semantics for declined enrollment.
• googlechat-network-policy-e2e (medium): The new Google Chat policy has security-sensitive method/path scoping and request_body_credential_rewrite behavior, but existing network-policy E2E does not prove this preset against chat.googleapis.com/www.googleapis.com routes.
  • Suggested test: Add a policy-focused Google Chat E2E or live policy probe that applies the googlechat preset and verifies allowed GET/POST/PATCH/DELETE paths under /v1/spaces/**, denial of unrelated methods/hosts, and credential rewrite only on chat.googleapis.com.

Dispatch hint

• Workflow: .github/workflows/e2e.yaml
• jobs input: messaging-providers,channels-add-remove,network-policy,credential-sanitization,tunnel-lifecycle,cloud-onboard

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

• channels-add-remove: The PR changes channel add/remove gateway-provider cleanup, built-in channel registration, messaging manifests, policy presets, bridge-provider wiring, and tunnel-backed enrollment behavior. The channels-add-remove live job is the smallest wired E2E job that exercises the real OpenClaw + Docker/OpenShell channel add, rebuild, policy-list, and channel removal cleanup path affected by these changes.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=channels-add-remove
• messaging-providers: The PR adds a new built-in messaging channel surface with provider profile, runtime preloads, manifest rendering, bridge-provider setup, and policy integration. The messaging-providers live job is the broad wired E2E coverage for messaging provider provisioning, placeholder/secret boundaries, rendered channel state, gateway policy, and live sandbox messaging runtime behavior.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=messaging-providers

Optional E2E targets

• messaging-compatible-endpoint: Optional adjacent coverage for messaging channel config/rendering and OpenClaw agent-turn behavior against a fake compatible endpoint. It does not directly exercise Google Chat, but it can catch regressions in shared messaging configuration and rendering paths touched by the PR.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=messaging-compatible-endpoint

Relevant changed files

• src/lib/actions/sandbox/policy-channel.ts
• src/lib/messaging/applier/build/messaging-build-applier.mts
• src/lib/messaging/channels/built-ins.ts
• src/lib/messaging/channels/googlechat/hooks/index.ts
• src/lib/messaging/channels/googlechat/hooks/tunnel-audience-gate.ts
• src/lib/messaging/channels/googlechat/hooks/tunnel-runtime.ts
• src/lib/messaging/channels/googlechat/manifest.ts
• src/lib/messaging/channels/googlechat/policy/openclaw.yaml
• src/lib/messaging/channels/googlechat/provider-profile/openclaw.yaml
• src/lib/messaging/channels/googlechat/rendered-config-parser.ts
• src/lib/messaging/channels/googlechat/runtime/googlechat-outbound-auth.ts
• src/lib/messaging/channels/googlechat/runtime/googlechat-trusted-proxy-fetch.ts
• src/lib/messaging/channels/googlechat/template-resolver.ts
• src/lib/messaging/channels/rendered-config-parser.ts
• src/lib/messaging/channels/template-resolver.ts
• src/lib/messaging/hooks/builtins.ts
• src/lib/onboard/messaging-bridge-provider.ts
• src/lib/onboard/messaging-prep.ts
• src/lib/onboard/providers.ts
• src/lib/tunnel/services.ts