Skip to content

fix: ensure NOTION_TOKEN takes priority for authentication#98

Open
bjh3311 wants to merge 1 commit intomakenotion:mainfrom
bjh3311:main
Open

fix: ensure NOTION_TOKEN takes priority for authentication#98
bjh3311 wants to merge 1 commit intomakenotion:mainfrom
bjh3311:main

Conversation

@bjh3311
Copy link
Copy Markdown

@bjh3311 bjh3311 commented Aug 2, 2025

The current implementation prioritizes OPENAPI_MCP_HEADERS over NOTION_TOKEN, even though the documentation lists NOTION_TOKEN as “Option 1 (recommended)”.

This PR is...

  • Aligns code behavior with the official documentation
  • Improves readability and performance by applying early return when NOTION_TOKEN is available
  • Keeps backward compatibility for users who rely on OPENAPI_MCP_HEADERS

@bjh3311
fix: ensure NOTION_TOKEN takes priority for authentication
1fbc2ff 
 Documentation shows NOTION_TOKEN as the recommended approach, but code was checking OPENAPI_MCP_HEADERS first

 It results in unnecessary performance overhead for most users
travbuddy
travbuddy approved these changes Aug 13, 2025
View reviewed changes
Copy link
Copy Markdown

@travbuddy travbuddy left a comment

Choose a reason for hiding this comment

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

looks good, let's get this merged 👍

@tech-sumit
Copy link
Copy Markdown

tech-sumit commented Aug 22, 2025 •
edited
Loading

Whoever can please merge this PR

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

Reviewers

2 more reviewers

@tech-sumit tech-sumit tech-sumit approved these changes

@travbuddy travbuddy travbuddy approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bjh3311 @tech-sumit @travbuddy