Skip to content

Conversation

@soheimam
Copy link
Collaborator

What changed? Why?
--Added timeout call out for notification
-- added depreciated warning for addMiniApp hook

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Oct 22, 2025

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

dylsteck
dylsteck previously approved these changes Oct 22, 2025
Copy link
Contributor

@dylsteck dylsteck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a non-blocking comment but otherwise LGTM

@dylsteck dylsteck self-requested a review October 23, 2025 17:57
@cb-heimdall cb-heimdall dismissed dylsteck’s stale review October 23, 2025 18:20

Approved review 3367372432 from dylsteck is now dismissed due to new commit. Re-request for approval.

"event": "notifications_disabled"
}
```
## Client App Behavior
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to the top under Overview

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Farcaster app: Activates notification tokens immediately without waiting for a webhook success response.

Base app: Waits for a successful webhook response before activating tokens.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then let's turn the last paragraph into a <Note> saying: If your webhook performs token saving and notification sending before returning a response, tokens may work on Farcaster but fail to activate on Base.


Different client apps handle webhook responses differently:

Farcaster app: Activates notification tokens immediately without waiting for a webhook success response.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add the bold back to each of these Farcaster App and Base App

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

capital A's as well


Different client apps handle webhook responses differently:

Farcaster app: Activates notification tokens immediately without waiting for a webhook success response.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

capital A's as well

@hughescoin hughescoin merged commit cfed850 into master Oct 24, 2025
8 checks passed
@hughescoin hughescoin deleted the docs/notifications-update branch October 24, 2025 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants