meet: chat-opportunity detector with regex pre-filter + Haiku confirmation#25943
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2542f424c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| this.onOpportunity(decision.reason); | ||
| } catch (err) { |
There was a problem hiding this comment.
If a Tier 2 LLM call is in flight and dispose() is called (for example when a meeting is being torn down), the promise can still resolve and execute this callback because there is no active/disposed check after await this.callDetectorLLM(...). That allows stale escalations to fire after teardown, which can trigger proactive-chat actions for a meeting that has already ended once this detector is wired into session management.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| .boolean({ | ||
| error: "services.meet.proactiveChat.enabled must be a boolean", | ||
| }) | ||
| .default(true) |
There was a problem hiding this comment.
🚩 proactiveChat.enabled defaults to true — behavioral change for existing Meet users when wired
The proactiveChat.enabled field defaults to true (skills/meet-join/config-schema.ts:134). When PR 7 wires the detector into the session manager, existing users who have services.meet.enabled: true will have proactive chat automatically enabled without explicit opt-in. Currently this is inert (no production code instantiates MeetChatOpportunityDetector), so there's no immediate impact. However, the wiring PR should either: (a) keep this default and document it in UPDATES.md, or (b) change the default to false so proactive chat is opt-in. The choice depends on product intent — flagging for the wiring PR author to decide.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Follow-up shipped in #26265 — addresses consolidated review feedback (chat concurrency mutex, dispose teardown, wake adapter timeout/ghost prevention, tokenEstimationProvider forwarding, E2E test flakiness). |
1 participant
Summary
Part of plan: meet-phase-2-chat.md (PR 5 of 8)