-
Notifications
You must be signed in to change notification settings - Fork 25
Feat : Add Notion Assistant AgentKit #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Hey @amanintech , kindly review my PR. |
📝 WalkthroughWalkthroughThis PR introduces a new Notion Assistant AgentKit template with project configuration, package manifest, documentation, and a core orchestration module that enables querying and managing Notion workspaces via API integration. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Orchestrate as orchestrate.ts
participant NotionClient as Notion Client
participant NotionAPI as Notion API
Note over Orchestrate: Module Initialization
Orchestrate->>Orchestrate: Load & validate env vars
Orchestrate->>NotionClient: Initialize with API key
NotionClient->>NotionAPI: Test connection
NotionAPI-->>NotionClient: Return database metadata
NotionClient-->>Orchestrate: Connection success
Note over Client,NotionAPI: Query Execution
Client->>Orchestrate: handleQuery("list databases")
Orchestrate->>NotionAPI: Search databases
NotionAPI-->>Orchestrate: Return database list
Orchestrate-->>Client: [{name, id}, ...]
Client->>Orchestrate: handleQuery("create page")
Orchestrate->>NotionAPI: Create page in database
NotionAPI-->>Orchestrate: Return pageId
Orchestrate-->>Client: {success: true, pageId}
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @templates/notionassistant/lamatic-config.json:
- Line 4: The config lists only "NOTION_API_KEY" under the api_keys_required
array but orchestrate.ts expects both NOTION_API_KEY and DATABASE_ID; update the
"api_keys_required" value to include "DATABASE_ID" alongside "NOTION_API_KEY" so
the lamatic-config.json explicitly declares both required environment keys
referenced by orchestrate.ts.
In @templates/notionassistant/package.json:
- Line 7: The dev script fails under "type": "module" because ts-node needs ESM
support; update the "dev" npm script that runs actions/orchestrate.ts to use an
ESM-aware runner (e.g., change the script to use "ts-node --esm
actions/orchestrate.ts" or "ts-node-esm actions/orchestrate.ts") or add a
ts-node config entry in tsconfig.json (e.g., "ts-node": { "esm": true }) so
ts-node runs correctly with ESM modules.
In @templates/notionassistant/README.md:
- Line 53: Replace the bold-format section titles with proper Markdown headings:
change "**List databases**" to a heading (e.g., "### List databases") and change
the bold "Create a new page" to "### Create a new page" so these become semantic
headings; update the two occurrences referenced by their text content ("List
databases" and "Create a new page") and keep the surrounding code blocks
unchanged.
🧹 Nitpick comments (3)
templates/notionassistant/actions/orchestrate.ts (3)
50-51: Side effect on module import.Calling
testNotionConnection()at module load causes a network request whenever this module is imported. This can cause issues in testing, build processes, or when the module is imported but the function isn't needed.Consider deferring connection test
Either export the function for explicit invocation or wrap it in a conditional:
-// Run the test immediately -testNotionConnection(); +// Export for explicit invocation when needed +export { testNotionConnection };Alternatively, if auto-connection is desired, gate it:
-// Run the test immediately -testNotionConnection(); +// Run the test only in runtime (not during imports for testing) +if (process.env.NODE_ENV !== 'test') { + testNotionConnection(); +}
66-80: Hardcoded page properties limit flexibility.The "create page" command uses hardcoded property names (
Name,Priority) and values. This assumes the target database has these exact properties configured, which may not always be the case.Consider accepting parameters for the page title and properties, or document that the database must have
Name(title) andPriority(select) properties configured.
34-34: Excessiveas anytype assertions.Multiple
as anycasts bypass TypeScript's type checking. While sometimes necessary for API quirks, this reduces type safety.Consider using proper Notion SDK types or creating typed interfaces. For example, the Notion SDK exports types like
DatabaseObjectResponsethat can be used instead ofany.Also applies to: 57-57, 74-75
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsontemplates/notionassistant/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.gitignorepackage.jsontemplates/notionassistant/README.mdtemplates/notionassistant/actions/orchestrate.tstemplates/notionassistant/app/page.tsxtemplates/notionassistant/lamatic-config.jsontemplates/notionassistant/lib/lamatic-client.tstemplates/notionassistant/package.jsontemplates/notionassistant/public/assests/asset.txt
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
templates/notionassistant/README.md
53-53: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
59-59: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (4)
.gitignore (1)
1-21: LGTM! Well-structured ignore patterns.The
.gitignorefile includes all essential patterns for a Node.js/TypeScript project:
- Prevents committing dependencies (
node_modules/)- Protects sensitive credentials (
.env)- Excludes build artifacts (
dist/,build/)- Filters logs and IDE-specific files
This aligns well with the Notion Assistant AgentKit requirements and security best practices.
package.json (1)
1-9: The version concern for@types/nodeis unfounded; consider the optional metadata addition.All specified versions are valid and available on npm:
@notionhq/client@5.3.0exists (latest: 5.6.0)dotenv@17.2.3is the current latest version@types/node@24.8.1is not unusually high—it's in the middle of the 24.x series, with newer versions available up to 25.0.6Adding standard package.json fields (
name,version,description,private) remains a best practice for tooling compatibility and clarity:📦 Optional: Add standard package.json fields
{ + "name": "agentkit-notion-workspace", + "version": "1.0.0", + "description": "Shared dependencies for Notion Assistant AgentKit", + "private": true, "dependencies": { "@notionhq/client": "^5.3.0", "dotenv": "^17.2.3" }, "devDependencies": { "@types/node": "^24.8.1" } }Likely an incorrect or invalid review comment.
templates/notionassistant/package.json (2)
15-17: Verify if Next.js and React dependencies are needed.The
orchestrate.tsmodule doesn't appear to use Next.js or React. If this AgentKit is purely a backend Notion integration, these dependencies may be unnecessary and bloat the package.Are there other files (e.g., UI components) that require these dependencies? If not, consider removing them.
21-21: [No action required]TypeScript 5.9.3 is the latest stable version on npm. The
^5.9.3constraint is appropriate and current.Likely an incorrect or invalid review comment.
| export async function handleQuery(query: string) { | ||
| if (query.toLowerCase().includes("list databases")) { | ||
| const res = await notion.search({ | ||
| filter: { property: "object", value: "database" } as any, | ||
| }); | ||
|
|
||
| return res.results.map((db: any) => ({ | ||
| name: db?.title?.[0]?.plain_text || "Untitled DB", | ||
| id: db.id, | ||
| })); | ||
| } | ||
|
|
||
| if (query.toLowerCase().includes("create page")) { | ||
| // Use non-null assertion directly for TypeScript | ||
| const res = await notion.pages.create({ | ||
| parent: { database_id: DATABASE_ID! }, | ||
| properties: { | ||
| // Cast to any to satisfy strict TypeScript types | ||
| Name: { | ||
| title: [{ text: { content: "New Task from Assistant AgentKit" } }], | ||
| } as any, | ||
| Priority: { select: { name: "High" } } as any, | ||
| }, | ||
| }); | ||
|
|
||
| return { success: true, pageId: res.id }; | ||
| } | ||
|
|
||
| return { message: "Command not recognized." }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling in handleQuery.
The handleQuery function doesn't wrap Notion API calls in try-catch blocks. If the API call fails (network error, rate limit, invalid permissions), the error will propagate unhandled, potentially crashing the application.
Suggested fix with error handling
export async function handleQuery(query: string) {
+ try {
if (query.toLowerCase().includes("list databases")) {
const res = await notion.search({
filter: { property: "object", value: "database" } as any,
});
return res.results.map((db: any) => ({
name: db?.title?.[0]?.plain_text || "Untitled DB",
id: db.id,
}));
}
if (query.toLowerCase().includes("create page")) {
- // Use non-null assertion directly for TypeScript
const res = await notion.pages.create({
parent: { database_id: DATABASE_ID! },
properties: {
- // Cast to any to satisfy strict TypeScript types
Name: {
title: [{ text: { content: "New Task from Assistant AgentKit" } }],
} as any,
Priority: { select: { name: "High" } } as any,
},
});
return { success: true, pageId: res.id };
}
return { message: "Command not recognized." };
+ } catch (err: any) {
+ return { success: false, error: err.message || "Notion API error" };
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function handleQuery(query: string) { | |
| if (query.toLowerCase().includes("list databases")) { | |
| const res = await notion.search({ | |
| filter: { property: "object", value: "database" } as any, | |
| }); | |
| return res.results.map((db: any) => ({ | |
| name: db?.title?.[0]?.plain_text || "Untitled DB", | |
| id: db.id, | |
| })); | |
| } | |
| if (query.toLowerCase().includes("create page")) { | |
| // Use non-null assertion directly for TypeScript | |
| const res = await notion.pages.create({ | |
| parent: { database_id: DATABASE_ID! }, | |
| properties: { | |
| // Cast to any to satisfy strict TypeScript types | |
| Name: { | |
| title: [{ text: { content: "New Task from Assistant AgentKit" } }], | |
| } as any, | |
| Priority: { select: { name: "High" } } as any, | |
| }, | |
| }); | |
| return { success: true, pageId: res.id }; | |
| } | |
| return { message: "Command not recognized." }; | |
| } | |
| export async function handleQuery(query: string) { | |
| try { | |
| if (query.toLowerCase().includes("list databases")) { | |
| const res = await notion.search({ | |
| filter: { property: "object", value: "database" } as any, | |
| }); | |
| return res.results.map((db: any) => ({ | |
| name: db?.title?.[0]?.plain_text || "Untitled DB", | |
| id: db.id, | |
| })); | |
| } | |
| if (query.toLowerCase().includes("create page")) { | |
| const res = await notion.pages.create({ | |
| parent: { database_id: DATABASE_ID! }, | |
| properties: { | |
| Name: { | |
| title: [{ text: { content: "New Task from Assistant AgentKit" } }], | |
| } as any, | |
| Priority: { select: { name: "High" } } as any, | |
| }, | |
| }); | |
| return { success: true, pageId: res.id }; | |
| } | |
| return { message: "Command not recognized." }; | |
| } catch (err: any) { | |
| return { success: false, error: err.message || "Notion API error" }; | |
| } | |
| } |
| { | ||
| "name": "notionassistant", | ||
| "description": "A Notion Assistant AgentKit to query and manage your Notion workspace.", | ||
| "api_keys_required": ["NOTION_API_KEY"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing DATABASE_ID in required keys.
The orchestrate.ts module requires both NOTION_API_KEY and DATABASE_ID environment variables, but only NOTION_API_KEY is listed here. Consider adding DATABASE_ID to ensure users know all required configuration:
Suggested fix
- "api_keys_required": ["NOTION_API_KEY"],
+ "api_keys_required": ["NOTION_API_KEY", "DATABASE_ID"],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "api_keys_required": ["NOTION_API_KEY"], | |
| "api_keys_required": ["NOTION_API_KEY", "DATABASE_ID"], |
🤖 Prompt for AI Agents
In @templates/notionassistant/lamatic-config.json at line 4, The config lists
only "NOTION_API_KEY" under the api_keys_required array but orchestrate.ts
expects both NOTION_API_KEY and DATABASE_ID; update the "api_keys_required"
value to include "DATABASE_ID" alongside "NOTION_API_KEY" so the
lamatic-config.json explicitly declares both required environment keys
referenced by orchestrate.ts.
| "type": "module", | ||
| "description": "A Notion Assistant AgentKit to query and manage your Notion workspace.", | ||
| "scripts": { | ||
| "dev": "ts-node actions/orchestrate.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESM + ts-node compatibility concern.
With "type": "module", running ts-node actions/orchestrate.ts directly may fail. ESM support in ts-node requires either the --esm flag or using ts-node-esm.
Suggested fix
- "dev": "ts-node actions/orchestrate.ts",
+ "dev": "ts-node --esm actions/orchestrate.ts",Alternatively, ensure tsconfig.json includes appropriate ts-node configuration for ESM.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dev": "ts-node actions/orchestrate.ts", | |
| "dev": "ts-node --esm actions/orchestrate.ts", |
🤖 Prompt for AI Agents
In @templates/notionassistant/package.json at line 7, The dev script fails under
"type": "module" because ts-node needs ESM support; update the "dev" npm script
that runs actions/orchestrate.ts to use an ESM-aware runner (e.g., change the
script to use "ts-node --esm actions/orchestrate.ts" or "ts-node-esm
actions/orchestrate.ts") or add a ts-node config entry in tsconfig.json (e.g.,
"ts-node": { "esm": true }) so ts-node runs correctly with ESM modules.
|
|
||
| ## Example Queries | ||
|
|
||
| **List databases** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use proper heading syntax instead of bold text.
Lines 53 and 59 use bold formatting for what should be semantic headings. This affects document structure and accessibility.
📝 Fix markdown heading syntax
-**List databases**
+### List databases
```javascript
await handleQuery("list databases");-Create a new page
+### Create a new page
await handleQuery("create page");</details>
Based on static analysis hints (markdownlint MD036).
Also applies to: 59-59
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
53-53: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @templates/notionassistant/README.md at line 53, Replace the bold-format
section titles with proper Markdown headings: change "List databases" to a
heading (e.g., "### List databases") and change the bold "Create a new page" to
"### Create a new page" so these become semantic headings; update the two
occurrences referenced by their text content ("List databases" and "Create a new
page") and keep the surrounding code blocks unchanged.
</details>
<!-- fingerprinting:phantom:triton:puma -->
<!-- This is an auto-generated comment by CodeRabbit -->
Description
This PR adds a Notion Assistant AgentKit that allows users to connect their Notion account and interact with their workspace conversationally.
The agent can fetch information, create pages, and summarize data using the Notion API.
Type of Change
Testing
Hacktoberfest
hacktoberfesttag to this PRChecklist
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.