feat(meetings): calendar-triggered auto-join prompt#3522
Closed
Sathvik-1007 wants to merge 10 commits into
Closed
Conversation
- fix: extract hangoutLink separately from htmlLink in collectors - add meet_url field to PendingEvent - new calendar.rs: identify_meet_meetings filter - planner evaluates auto_join_policy per meeting event - new RPC: agent_meetings_notification_action (join/skip/always) - frontend: pass actions through socket, render action buttons - MeetingAutoJoinTriggered domain event for Always policy Closes tinyhumansai#3507
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds end-to-end meeting auto-join prompt support: ChangesMeeting Notification Actions
CI Coverage Cleanup
Sequence Diagram(s)sequenceDiagram
rect rgba(200, 230, 255, 0.5)
note over Planner: Backend heartbeat tick
Planner->>CalendarIdentifier: identify_meet_meetings(pending_events)
CalendarIdentifier-->>Planner: Vec<CalendarMeeting>
Planner->>MeetingStore: create/get MeetingSession (idempotent)
alt auto_join_policy == AskEachTime
Planner->>EventBus: publish CoreNotificationEvent with join/skip/always_join actions
else auto_join_policy == Always or Never
Planner->>EventBus: publish MeetingAutoJoinTriggered
end
end
rect rgba(230, 255, 200, 0.5)
note over Socket,Notifications: Frontend
EventBus->>Socket: core_notification {actions}
Socket->>NotificationService: forward with actions
NotificationService->>Notifications: dispatch notification item with actions
Notifications->>Notifications: render action buttons
Notifications->>RPCHandler: callCoreRpc agent_meetings_notification_action {action_id, payload}
RPCHandler->>MeetingStore: update session status
RPCHandler-->>Notifications: ok true
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
app/src/lib/nativeNotifications/service.ts (1)
42-42: ⚡ Quick winUse a named
interfacefor notification actions.The inline
{ actionId; label; payload? }object type should be extracted to aninterfacefor guideline compliance.As per coding guidelines,
**/*.{ts,tsx}should useinterfacefor defining object shapes in TypeScript.Suggested fix
+interface CoreNotificationAction { + actionId: string; + label: string; + payload?: unknown; +} + interface CoreNotificationPayload { id: string; category: NotificationCategory; title: string; body: string; deep_link?: string | null; timestamp_ms: number; - actions?: Array<{ actionId: string; label: string; payload?: unknown }>; + actions?: CoreNotificationAction[]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/lib/nativeNotifications/service.ts` at line 42, Extract the inline object type used for notification actions into a named interface (e.g., declare interface NotificationAction { actionId: string; label: string; payload?: unknown }) and then replace the inline type in the `actions?: Array<{ actionId: string; label: string; payload?: unknown }>;` declaration with `actions?: NotificationAction[]`; update any imports/exports or other references in the same file so the new `NotificationAction` interface is used consistently (look for `actions` in service.ts and any related types in this module).Source: Coding guidelines
src/openhuman/agent_meetings/ops.rs (1)
291-377: ⚡ Quick winAdd debug diagnostics to the new notification-action flow.
This new/changed flow currently has no entry/branch/error tracing, which makes action failures hard to debug.
As per coding guidelines, new/changed Rust flows under
src/openhuman/**/*.rsshould include debug/trace logging for entry/exit, branches, and errors with stable prefixes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent_meetings/ops.rs` around lines 291 - 377, The handler lacks diagnostics: update handle_notification_action to emit stable-prefixed debug/trace logs at entry (log action_id, meeting_id, meet_url and timestamp), before each branch (log entering "join_meeting", "skip_meeting", "always_join"), and on all error paths and after successful outcomes; use the repository logging facility (e.g., tracing::debug/trace/error) with a consistent prefix like "[agent_meetings]" and include unique identifiers (action_id, meeting_id, correlation_id) and the function name handle_notification_action so failures and branch decisions are observable; also log config load/save and store::update_session_status results and the final returned outcome.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/pages/Notifications.tsx`:
- Around line 168-180: The action buttons currently render raw action.label;
change them to use the localization hook useT() and translate by actionId (e.g.,
t(`notifications.actions.${action.actionId}`)), ensuring useT is imported/used
in Notifications (or the component rendering item.actions) and provide a
fallback to action.label or a sensible default if the translation key is
missing; keep onclick behavior (handleAction(item, action.actionId,
action.payload)) unchanged.
- Around line 61-74: handleAction is vulnerable to duplicate RPCs when the user
clicks repeatedly; add an in-flight guard (e.g., a component-scoped Set stored
in a ref like pendingActionsRef) and use a composite key
`${item.id}:${actionId}` to short-circuit and return immediately if the key is
already pending, add the key to the set before awaiting callCoreRpc, and ensure
the key is removed in a finally block so retries are possible; apply the same
pattern to the other handler at the 171-177 block and keep using markRead and
callCoreRpc as currently named.
In `@src/openhuman/agent_meetings/ops.rs`:
- Around line 326-335: The code sets session status to
MeetingSessionStatus::Joined via super::store::update_session_status before
awaiting handle_join, which can leave state inconsistent if handle_join fails;
change the flow so you first call handle_join(join_params).await and only upon
its Ok result call super::store::update_session_status(&config, &meeting_id,
super::types::MeetingSessionStatus::Joined, now_ms), propagating errors from
update_session_status as before; apply the same fix to the other occurrence
referenced (the block around the handle_join call at the 365-374 region) so both
join paths update status only after handle_join succeeds.
- Around line 299-308: Current code defaults payload fields meeting_id and
meet_url to empty strings which masks missing required data; update the handler
logic that reads meeting_id and meet_url so it returns an explicit error when
required fields are absent instead of unwrap_or(""), validating per action:
require meeting_id for the "skip" action and require both meeting_id and
meet_url for "join" and "always_join" actions; locate the occurrences where
meeting_id and meet_url are extracted (variables named meeting_id and meet_url
around the blocks handling skip/join/always_join and the repeated occurrences at
the other extraction sites) and replace the unwrap_or("") extraction with proper
Option checks that produce an Err or early-return with a clear error message
indicating the missing field(s).
In `@src/openhuman/subconscious/heartbeat/planner/mod.rs`:
- Around line 155-157: The auto-join dispatch is being triggered unconditionally
by build_meeting_actions and publishes MeetingAutoJoinTriggered for
AutoJoin::Always too early and possibly multiple times; update the logic so that
build_meeting_actions (or its caller) only considers and publishes
MeetingAutoJoinTriggered when the event.stage corresponds to the "happening now"
or "final_call" states (e.g., match the event.stage enum cases used elsewhere),
and add an idempotency guard before publishing (check an existing flag/marker on
the event or ensure the action list does not already contain
MeetingAutoJoinTriggered) so the event only fires once per meeting.
- Around line 235-242: The code currently treats any error from
meetings_store::create_session(config, &session) as a benign "already exists"
case; instead, match the returned error (from meetings_store::create_session) to
distinguish a UNIQUE-constraint/duplicate-session error from other DB failures:
if it's the duplicate/unique error, keep the existing tracing::debug for
meeting_id and continue, but for any other error log it at error level and
propagate/fail early (return Err or bail) so you do not emit buttons/events tied
to meeting_id. Locate meetings_store::create_session and the surrounding planner
code that uses meeting_id and session and implement this error-kind check (use
the concrete error variant/type exported by meetings_store or inspect SQL error
code) before proceeding.
---
Nitpick comments:
In `@app/src/lib/nativeNotifications/service.ts`:
- Line 42: Extract the inline object type used for notification actions into a
named interface (e.g., declare interface NotificationAction { actionId: string;
label: string; payload?: unknown }) and then replace the inline type in the
`actions?: Array<{ actionId: string; label: string; payload?: unknown }>;`
declaration with `actions?: NotificationAction[]`; update any imports/exports or
other references in the same file so the new `NotificationAction` interface is
used consistently (look for `actions` in service.ts and any related types in
this module).
In `@src/openhuman/agent_meetings/ops.rs`:
- Around line 291-377: The handler lacks diagnostics: update
handle_notification_action to emit stable-prefixed debug/trace logs at entry
(log action_id, meeting_id, meet_url and timestamp), before each branch (log
entering "join_meeting", "skip_meeting", "always_join"), and on all error paths
and after successful outcomes; use the repository logging facility (e.g.,
tracing::debug/trace/error) with a consistent prefix like "[agent_meetings]" and
include unique identifiers (action_id, meeting_id, correlation_id) and the
function name handle_notification_action so failures and branch decisions are
observable; also log config load/save and store::update_session_status results
and the final returned outcome.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3197e9e8-f844-4ec7-b8e1-aa4dd05f53f7
📒 Files selected for processing (10)
app/src/lib/nativeNotifications/service.tsapp/src/pages/Notifications.tsxsrc/core/event_bus/events.rssrc/openhuman/agent_meetings/calendar.rssrc/openhuman/agent_meetings/mod.rssrc/openhuman/agent_meetings/ops.rssrc/openhuman/agent_meetings/schemas.rssrc/openhuman/subconscious/heartbeat/planner/collectors.rssrc/openhuman/subconscious/heartbeat/planner/mod.rssrc/openhuman/subconscious/heartbeat/planner/types.rs
…stage gate - move session status update after handle_join succeeds - validate meeting_id/meet_url presence, return error if missing - gate auto-join dispatch to final_call/happening_now stages - add tracing::debug to notification_action handler - fix E0603: use re-exported AutoJoinPolicy path - extract NotificationAction interface in service.ts - add in-flight guard to prevent duplicate action RPCs
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/pages/Notifications.tsx (1)
62-78:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd user feedback for RPC failures.
The RPC call at line 69 has no error handling. When invoked via
void handleAction(...)on line 180, any RPC failure will be silently swallowed, leaving the user with no indication whether their action (join/skip/always join) succeeded or failed.💬 Proposed fix: add try-catch and user feedback
const handleAction = useCallback( async (item: NotificationItem, actionId: string, payload?: unknown) => { const key = `${item.id}:${actionId}`; if (pendingActionsRef.current.has(key)) return; pendingActionsRef.current.add(key); if (!item.read) dispatch(markRead({ id: item.id })); try { await callCoreRpc({ method: 'openhuman.agent_meetings_notification_action', params: { notification_id: item.id, action_id: actionId, payload }, }); + // Optional: show success toast + } catch (err) { + console.error('[Notifications] action failed', err); + // TODO: show error toast to user } finally { pendingActionsRef.current.delete(key); } }, [dispatch] );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/Notifications.tsx` around lines 62 - 78, The handleAction callback currently awaits callCoreRpc without handling errors so failures are silent; wrap the RPC call in a try-catch around callCoreRpc inside handleAction, log the error (console.error) and surface a user-visible error (e.g., dispatch a toast/notification or show an alert) mentioning the failed action and notification id, and if you made optimistic changes (dispatch(markRead({ id: item.id }))) consider reverting or informing the user on failure; keep the existing finally block that deletes the key from pendingActionsRef.current to ensure cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/src/pages/Notifications.tsx`:
- Around line 62-78: The handleAction callback currently awaits callCoreRpc
without handling errors so failures are silent; wrap the RPC call in a try-catch
around callCoreRpc inside handleAction, log the error (console.error) and
surface a user-visible error (e.g., dispatch a toast/notification or show an
alert) mentioning the failed action and notification id, and if you made
optimistic changes (dispatch(markRead({ id: item.id }))) consider reverting or
informing the user on failure; keep the existing finally block that deletes the
key from pendingActionsRef.current to ensure cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59323233-a5dc-4d6f-8be7-0df2c4d38d6c
📒 Files selected for processing (4)
app/src/lib/nativeNotifications/service.tsapp/src/pages/Notifications.tsxsrc/openhuman/agent_meetings/ops.rssrc/openhuman/subconscious/heartbeat/planner/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/lib/nativeNotifications/service.ts
- src/openhuman/subconscious/heartbeat/planner/mod.rs
- src/openhuman/agent_meetings/ops.rs
coderabbitai
Bot
added
the
agent
7 tasks
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
Author
|
@coderabbitai resolve |
Contributor
✅ Action performedComments resolved and changes approved. |
Collaborator
|
Thanks for the contribution! Closing this as a duplicate — the same change is already on If your version covers something the merged PR doesn't, just comment and we'll gladly reopen. 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Calendar watch + auto-join prompt for Google Meet meetings. Depends on PR-0 (#3516, merged).
Changes
Bug fix:
collect_calendar_meetingsstoredhtmlLinkindeep_linkwhen both links existed — Meet URL was lost. Now extractshangoutLinkseparately intoPendingEvent.meet_url.New:
agent_meetings/calendar.rs—identify_meet_meetingsfilters events whosemeet_urlmatches allowed conferencing hosts.Planner policy routing: For meetings with a
meet_url, creates aMeetingSession(Pending) and evaluatesauto_join_policy:Never— no action buttonsAlways— emitsMeetingAutoJoinTriggereddomain event (transparency notification, no buttons)AskEachTime— publishes notification with Add OpenHuman / Not this one / Always join buttonsNew RPC:
openhuman.agent_meetings_notification_actionhandles user clicks:join_meeting— joins in listen-only mode with correlation_idskip_meeting— transitions session to Endedalways_join— flips config to Always + joinsFrontend: Socket listener passes
actionsfromcore_notificationevents. Notifications page renders action buttons and calls the new RPC.Testing
Validation Run
cargo fmt --all -- --checktsc --noEmit)Closes #3507
Summary by CodeRabbit