Skip to content

Fix Google Meet tool session ownership#72441

Merged
steipete merged 4 commits intoopenclaw:mainfrom
BsnizND:fix/google-meet-tool-gateway-ownership
Apr 27, 2026
Merged

Fix Google Meet tool session ownership#72441
steipete merged 4 commits intoopenclaw:mainfrom
BsnizND:fix/google-meet-tool-gateway-ownership

Conversation

@BsnizND
Copy link
Copy Markdown
Contributor

@BsnizND BsnizND commented Apr 27, 2026

Summary

  • Route stateful google_meet agent tool actions through the Google Meet gateway methods so create/join/status/leave/speak/test_speech/recover calls share the gateway-owned plugin runtime instead of creating tool-turn-local sessions.
  • Preserve structured gateway error details while keeping the legacy Google Meet error payload shape for manual-action responses.
  • Harden node-host command-pair cleanup so already-closed sessions still terminate child bridge processes, with SIGTERM then SIGKILL fallback.

Fixes #72440.

Validation

  • pnpm exec oxfmt --check extensions/google-meet/index.ts extensions/google-meet/index.test.ts extensions/google-meet/index.create.test.ts extensions/google-meet/src/test-support/plugin-harness.ts extensions/google-meet/src/node-host.ts
  • pnpm exec vitest run extensions/google-meet/index.test.ts extensions/google-meet/index.create.test.ts
  • pnpm tsgo:extensions:test
  • git diff --check

Live runtime smoke evidence

  • Jay tool-created room https://meet.google.com/krq-swpp-kik / meet_40603aaf-6587-4527-80a5-502b598f9e27 remained visible to googlemeet.status after the Jay tool turn ended, with providerConnected: true and realtimeReady: true.
  • Direct create/leave smoke https://meet.google.com/hcz-ftus-qhy / meet_7a5d75ee-b5dc-46ad-ab86-aaee35381cc7 left no lingering SoX bridge processes after googlemeet.leave.

Notes

This PR complements, rather than replaces, the earlier bridge-orphan and Gemini function-response fixes. The observed production failure was that sessions created by the agent tool path were not owned by the gateway process after the tool turn returned.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 27, 2026

Greptile Summary

Routes all stateful google_meet agent tool actions through the gateway-owned plugin runtime via callGoogleMeetGatewayFromTool, fixing the production issue where tool-turn-local sessions were not visible to the gateway after the turn ended. The error round-trip (errorShape in sendError + readGatewayErrorDetails in the tool path) correctly preserves the legacy error payload shape for agent responses, and the node-host SIGTERM→SIGKILL hardening ensures bridge processes are cleaned up even when stopSession is re-entered on an already-closed session.

Confidence Score: 4/5

Safe to merge — no P0/P1 issues; the core architectural change is correct and well-tested.

All changed paths are logically sound. The only finding is a P2 style inconsistency where the leave/speak gateway handlers call respond(false, ...) with two args (no errorShape) while every other failure path uses the new sendError helper. This is unreachable from the tool action due to pre-validation, so it has no runtime impact.

extensions/google-meet/index.ts — the pre-existing 2-arg respond(false, ...) calls in googlemeet.leave and googlemeet.speak handlers (lines 780–782, 797–799) are inconsistent with the new sendError pattern but are harmless.

Comments Outside Diff (1)

  1. extensions/google-meet/index.ts, line 775-789 (link)

    P2 leave/speak gateway error responses bypass sendError

    The leave (and speak) gateway handler calls respond(false, { error: "sessionId required" }) with only two arguments, omitting the errorShape third argument now used by sendError. This means a caller that hits this path gets an error without the structured ErrorCodes/details envelope. While the tool pre-validates sessionId so this code path is unreachable from callGoogleMeetGatewayFromTool, a caller reaching the gateway directly (e.g., CLI) gets an inconsistent error shape compared to every other failure path in this file.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: extensions/google-meet/index.ts
    Line: 775-789
    
    Comment:
    **`leave`/`speak` gateway error responses bypass `sendError`**
    
    The `leave` (and `speak`) gateway handler calls `respond(false, { error: "sessionId required" })` with only two arguments, omitting the `errorShape` third argument now used by `sendError`. This means a caller that hits this path gets an error without the structured `ErrorCodes`/`details` envelope. While the tool pre-validates `sessionId` so this code path is unreachable from `callGoogleMeetGatewayFromTool`, a caller reaching the gateway directly (e.g., CLI) gets an inconsistent error shape compared to every other failure path in this file.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/google-meet/index.ts
Line: 775-789

Comment:
**`leave`/`speak` gateway error responses bypass `sendError`**

The `leave` (and `speak`) gateway handler calls `respond(false, { error: "sessionId required" })` with only two arguments, omitting the `errorShape` third argument now used by `sendError`. This means a caller that hits this path gets an error without the structured `ErrorCodes`/`details` envelope. While the tool pre-validates `sessionId` so this code path is unreachable from `callGoogleMeetGatewayFromTool`, a caller reaching the gateway directly (e.g., CLI) gets an inconsistent error shape compared to every other failure path in this file.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Fix Google Meet tool session ownership" | Re-trigger Greptile

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 27, 2026

Codex automated review: keeping this open.

Keep this PR open. It is a focused contributor fix for the still-open same-author bug report #72440, and the PR body uses closing syntax for that issue, so it should remain available for maintainer review/merge rather than be auto-closed. Current main still has the core ownership gap: stateful google_meet tool actions call the plugin runtime directly inside the tool execution path, while the gateway-owned googlemeet.* handlers maintain their own runtime instance. The PR diff addresses that by routing those tool actions through fixed gateway methods and also tackles a still-relevant node-host cleanup edge case, though it likely needs rebase/integration against newer Google Meet changes on main.

Best possible solution:

Keep this PR open for maintainer review. Rebase it onto current main, preserve the newer Google Meet realtime/audio changes, consider tightening or documenting the internal gateway RPC auth mode, address the direct leave/speak error-shape consistency if practical, then run the focused Google Meet format/test/typecheck lanes before merging. Once merged, #72440 should close through the PR's existing closing syntax.

What I checked:

  • Paired open issue and closing syntax: The live GitHub API data shows this PR is open, authored by BsnizND, and its body says Fixes #72440; Google Meet tool-created realtime sessions can lose gateway ownership #72440 is also open and authored by BsnizND. The workflow rule says to keep an implementation PR/issue pair open until the PR is reviewed and merged or explicitly closed by a maintainer.
  • Current main still uses tool-local runtime for stateful actions: On current main, the google_meet tool execute switch calls ensureRuntime() and invokes rt.join, createAndJoinMeetFromParams, rt.testSpeech, rt.status, rt.recoverCurrentTab, rt.setupStatus, rt.leave, and rt.speak directly from the tool path. That matches the reported ownership problem rather than the PR's proposed gateway-owned path. (extensions/google-meet/index.ts:751, bfdee5fa72cc)
  • Gateway-owned methods already exist as the right target: Current main registers googlemeet.join, googlemeet.create, googlemeet.status, googlemeet.recoverCurrentTab, googlemeet.setup, googlemeet.leave, googlemeet.speak, and googlemeet.testSpeech against the plugin registration runtime, so routing stateful tool calls through these handlers is a plausible owner-boundary fix. (extensions/google-meet/index.ts:505, bfdee5fa72cc)
  • PR diff targets the reported ownership path: The PR adds callGoogleMeetGatewayFromTool, fixed mapping from tool actions to googlemeet.* gateway methods, structured error detail preservation, and replaces the stateful tool branches with gateway calls. The diff is scoped to Google Meet source/tests and test harness code. (extensions/google-meet/index.ts:287, d194a2e9f022)
  • Node-host cleanup remains relevant but overlaps newer main: Current main has SIGTERM/SIGKILL fallback in terminateChild, but stopSession still returns immediately when session.closed is true, which is the specific surviving-child cleanup edge case described in Google Meet tool-created realtime sessions can lose gateway ownership #72440 and in the PR body. Main also has newer clearAudio, closedAt, and output-process handling from commit 2785be2, so the PR's node-host hunk needs integration rather than closure. (extensions/google-meet/src/node-host.ts:105, bfdee5fa72cc)
  • Security review pass: The PR does not touch workflows, lockfiles, package metadata, install/build/release scripts, or downloaded artifacts. The security-sensitive change is the new gateway RPC call from a tool: it uses the existing openclaw/plugin-sdk/browser-node-runtime export for callGatewayFromCli, while current gateway call code enforces explicit credentials for URL overrides and blocks insecure non-loopback ws:// endpoints. Maintainers should still review whether CLI-mode/default operator scopes are the intended internal call mode. (src/cli/gateway-rpc.runtime.ts:6, bfdee5fa72cc)

Remaining risk / open question:

  • The PR is based on an older main SHA and overlaps newer Google Meet commits on current main, especially Fix Google Meet realtime interruption playback #72524 and Add Google Meet realtime consult agentId #72381, so it likely needs a careful rebase/integration before merge.
  • The gateway RPC path should be reviewed for least-privilege/auth semantics because callGatewayFromCli uses CLI client mode and CLI default operator scopes, even though the PR maps only fixed Google Meet methods and current gateway code has URL/auth safeguards.
  • Greptile's P2 note is valid for consistency: direct gateway leave and speak missing-sessionId failures still return a two-argument response without the structured error envelope introduced elsewhere in the PR.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against bfdee5fa72cc.

@steipete steipete force-pushed the fix/google-meet-tool-gateway-ownership branch 6 times, most recently from c154919 to 56feb25 Compare April 27, 2026 08:16
@steipete steipete force-pushed the fix/google-meet-tool-gateway-ownership branch from 56feb25 to 1cab15c Compare April 27, 2026 08:24
@steipete steipete merged commit 916eda1 into openclaw:main Apr 27, 2026
65 checks passed
@steipete
Copy link
Copy Markdown
Contributor

Landed via squash onto main.

  • Source head: 1cab15c
  • Landed commit: 916eda1
  • Gate: exact-head CI run 24984427442 passed on 1cab15c169ec4d036cf9ec98b1162fc3b771b574; Testbox targeted proof covered pnpm test extensions/google-meet/index.test.ts extensions/google-meet/index.create.test.ts, pnpm tsgo:extensions:test, pnpm check:architecture, and the follow-up registry mock fix proof pnpm test src/plugins/bundle-commands.test.ts.
  • Follow-up review fix: missing-session-id googlemeet.leave/googlemeet.speak paths now return structured gateway errors instead of raw legacy payloads.

Thanks @BsnizND!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Google Meet tool-created realtime sessions can lose gateway ownership

2 participants