Skip to content
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

fix: add context to prevent notion from creating unwanted pages #171

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

njhale
Copy link
Contributor

@njhale njhale commented Nov 6, 2024

Add context to the Notion tools to confirm with the user before creating new pages. This will prevent pages from being created when the user is actually attempting to update the content of an existing page.

This change also bumps node-gptscript to include the new dataset changes.

Addresses otto8-ai/otto8#251

@njhale
Copy link
Contributor Author

njhale commented Nov 6, 2024

Running into some dataset issues. Going to see if they're resolved with gptscript-ai/node-gptscript#102

@@ -7,7 +7,7 @@
"author": "",
"license": "Apache-2.0",
"dependencies": {
"@gptscript-ai/gptscript": "github:gptscript-ai/node-gptscript#b30db3cf4fe87b1ee09a20752baf2263b1e5c7be",
"@gptscript-ai/gptscript": "github:g-linville/node-gptscript#49125f86e4a0c2af02b85373fdf647699dfcd58b",
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather just wait until grant's changes are merged rather than using his fork in the primary tool repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally, I'm just testing this in advance since datasets are currently broken against the latest version of gptscript

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK -- the dataset changes have merged and I updated this dependency.

@njhale njhale force-pushed the fix/notion-i251-plusplus branch 10 times, most recently from ffb1d3b to ec9eee5 Compare November 6, 2024 22:19
@njhale njhale requested a review from drpebcak November 6, 2024 22:22
Copy link
Collaborator

@g-linville g-linville left a comment

Choose a reason for hiding this comment

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

This LGTM, but I would prefer for it to merge after my PR to this repo, which will conflict with this one but updates all tools in a standard way.

Add context to the Notion tools to confirm with the user before creating
new pages. This will prevent pages from being created when the user
is actually attempting to update the content of an existing page.

Signed-off-by: Nick Hale <[email protected]>
@njhale njhale merged commit 2e6a48c into otto8-ai:main Nov 7, 2024
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.

4 participants