Direct notifications from apps to omi with mentor example#1560
Conversation
|
One improvement that can be made is making an Omi SDK for the backend, currently there is one implemented as class FriendClient in plugins/example/zapier/client.py but it should be moved to /plugins and renamed to OmiClient or OmiSDK so apps can use all available backend functions, and add more like getting user facts etc. Also the auth needs to be implemented like I did in this PR, the current FriendClient SDK for Zapier uses a hardcoded WORKFLOW_API_KEY in the backed, so it cannot be used by app developers like I did in this PR. I suggest a bounty to make a proper Omi SDK for a wider backed API, using my auth method or an API key generator. |
|
Hi @tiagoefreitas, can we pls have your notification logic as a completely different endpoint and function instead of mixing it in |
|
@mdmohsin7 I moved it to /v1//integrations/notifications because integrations has other public apis used by the zapier app. @kodjima33 if you want me to do the SDK, please create a separate issue Also should I add the ability to use prompt templates in the direct notifications? Right now it only supports direct messages, which gives more control to devs, but prompts with the args may be easier for simpler apps. |
|
@mdmohsin7 @kodjima33 congrats on the new launch I get you are busy, can you review this PR and let me know? |
|
@kodjima33 any news on this? thanks |
|
man, @tiagoefreitas , you can do it better, please take a look: 1/ app_id is not strong enough for the authentication. suggest the new term: app's secret. we need to let dev create/revoke their app's secret from omi app. 2/ did you implement the rate limit for POST /v1/integrations/notification ? sry i can not see that piece of code, feel free to lmk. / draft |
|
@beastoin ok will implement that this week |
|
@beastoin I implemented your suggestions and tested, can you review? thanks |
|
Cool man / @tiagoefreitas 1/ the back-end should not store the apps secret key, {hashed + label) is the way. could you improve it ? |
|
@beastoin by label you mean support for multiple keys with user label? one key should be enough because its per app. ok will remove burst protection. it was meant to prevent developers from sending too many notifications to users, not for security. will make the demo video after these clarifications |
|
1/ label - sk-...NlQA, 1-1 for now. the why? developers can have multiple apps and key's label will help them take the action without further concerns. looking forward for the demo. |
|
ok but the key shows inside the app config. should the label be set by the
user or just the app name? currently I create the key when the app is
created if it has notifications and can be revoked, but if we have manual
label will have to be created manually
…On Tue, 4 Mar 2025 at 02:29, Thinh ***@***.***> wrote:
1/ label - sk-...NlQA, 1-1 for now. the why? developers can have multiple
apps and key's label will help them take the action without further
concerns.
looking forward for the demo.
@tiagoefreitas <https://github.com/tiagoefreitas>
—
Reply to this email directly, view it on GitHub
<#1560 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACXJNRVBHYYQJEIY5MR7HW32SUFYFAVCNFSM6AAAAABT5RDFCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOJWGAYTSNZYGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: beastoin]*beastoin* left a comment (BasedHardware/omi#1560)
<#1560 (comment)>
1/ label - sk-...NlQA, 1-1 for now. the why? developers can have multiple
apps and key's label will help them take the action without further
concerns.
looking forward for the demo.
@tiagoefreitas <https://github.com/tiagoefreitas>
—
Reply to this email directly, view it on GitHub
<#1560 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACXJNRVBHYYQJEIY5MR7HW32SUFYFAVCNFSM6AAAAABT5RDFCCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOJWGAYTSNZYGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
showing |
|
@beastoin here is the demo, hope its clear. |
|
1/ man, i have a gift for you, the apps api key(multiple keys) is ready on the main https://github.com/BasedHardware/omi/blob/main/backend/routers/apps.py#L822-L874 , please re-base your code with the main; you could also use this for your ticket / related doc https://docs.omi.me/docs/developer/apps/IntegrationActions 2/ ok 3/ ok looking forward to your changes. lets get this ticket done asap. |
|
@beastoin ok I will revert all the work on the api key, use yours and retest. you could have told me you were already working on it... |
|
@beastoin do you want me to move it to backend/routers/integration.py with route /v2/integrations/{app_id}/notification ? Right now its in backend/routers/notifications.py with /v1/integrations/notification because there was already a /v1/notification for admins |
yes pls @tiagoefreitas |
|
@beastoin done and retested, ready to merge |
|
lgtm @tiagoefreitas 🥳 it would be great if you could create a new PR to: 4/ move the rate-limit logic to utils(out of the routers) |
…are#1560) Implements BasedHardware#1434 The example in the mentor app sends a reminder to the user 1 minute after the last mentor notification saying "Hey! How's it going with my previous suggestion? Have you had a chance to try it out?". This could be improved further by merging PR BasedHardware#1440 , that way the app could save the last notification sent and create a personalized message reminding the user about the previous suggestion and asking how its going. For authentication: We send the app if to the app transcriptions webhook, the app should save the id. When the apps call the /notification webhook on the omi api, they need to provide the app base url (same as configure in the webhook) as the http referrer. And they need to send the uid and the aid (app id). If the app is not installed on that user or the url doesn't match, the request fails. We could implement more robust api key authentication, but it would make it more complex for the developers, and this is secure now because only the app can receive the app id from the backend. Effectively the app id serves as api key, that can be revoked by just uninstalling the app.
3 participants
Implements #1434
The example in the mentor app sends a reminder to the user 1 minute after the last mentor notification saying "Hey! How's it going with my previous suggestion? Have you had a chance to try it out?".
This could be improved further by merging PR #1440 , that way the app could save the last notification sent and create a personalized message reminding the user about the previous suggestion and asking how its going.
For authentication:
We send the app if to the app transcriptions webhook, the app should save the id.
When the apps call the /notification webhook on the omi api, they need to provide the app base url (same as configure in the webhook) as the http referrer.
And they need to send the uid and the aid (app id).
If the app is not installed on that user or the url doesn't match, the request fails.
We could implement more robust api key authentication, but it would make it more complex for the developers, and this is secure now because only the app can receive the app id from the backend. Effectively the app id serves as api key, that can be revoked by just uninstalling the app.