Skip to content

Add Google Keep Integration - #499

Open
hGriff0n wants to merge 10 commits into
taylorwilsdon:mainfrom
hGriff0n:keep
Open

Add Google Keep Integration#499
hGriff0n wants to merge 10 commits into
taylorwilsdon:mainfrom
hGriff0n:keep

Conversation

@hGriff0n

@hGriff0n hGriff0n commented Feb 21, 2026

Copy link
Copy Markdown

Description

Implements #481 to include the Google Keep API (https://developers.google.com/workspace/keep/api/reference/rest)

Adds the following tools:

  • get_note: read the note and metadata
  • read_note: read the note's contents (supports text and checklist modes)
  • delete_note: delete the note
  • list_notes: list all notes
  • download_attachment: downloads the indicated attachment
  • set_permissions: updates the writer permissions on the note

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this change manually

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have enabled "Allow edits from maintainers" for this pull request

⚠️ IMPORTANT: This repository requires that you enable "Allow edits from maintainers" when creating your pull request. This allows maintainers to make small fixes and improvements directly to your branch, speeding up the review process.

To enable this setting:

  1. When creating the PR, check the "Allow edits from maintainers" checkbox
  2. If you've already created the PR, you can enable this in the PR sidebar under "Allow edits from maintainers"

Summary by CodeRabbit

  • New Features

    • Google Keep support: create, read, list, delete notes (text & checklists), attachments, permissions, pagination/filtering, and both full-content and concise summaries.
  • Documentation

    • CLI, manifest, UI/tool listings and help text updated to include Keep.
  • Tests

    • Comprehensive unit tests added for Keep tools and helpers (create, read, list, delete, attachments, permissions, helpers).

@coderabbitai

coderabbitai Bot commented Feb 21, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds Google Keep integration: new OAuth scopes, service registration, data models and helpers, async Keep tools (list/get/read/create/delete/download/set_permissions), CLI/manifest/helm/smithery registrations, core infra updates, log prefix, tool tiers, and comprehensive tests.

Changes

Cohort / File(s) Summary
Auth & Service Config
auth/scopes.py, auth/service_decorator.py
Add KEEP_READONLY_SCOPE, KEEP_WRITE_SCOPE, KEEP_SCOPES; update SCOPE_HIERARCHY, TOOL_SCOPES_MAP, TOOL_READONLY_SCOPES_MAP; register keep service and expose new scope groups.
Keep Implementation
gkeep/__init__.py, gkeep/keep_helpers.py, gkeep/keep_tools.py
New gkeep package: dataclasses (ListItem, Attachment, Permission, Note), helpers (format_reauth_message, format_note, format_note_content, build_note_body), and async tools (list/get/read/create/delete/download_attachment/set_permissions).
Server, CLI & Manifest
main.py, fastmcp_server.py, manifest.json, helm-chart/.../values.yaml, smithery.yaml
Register keep in CLI --tools, tool imports/icons, server all_services, manifest tools/keywords, and Helm/smithery listings.
Core infra adjustments
core/api_enablement.py, core/log_formatter.py, core/tool_tiers.yaml
Replace static enablement links with SUPPORTED_APIS/API_ENABLEMENT_LINK_BASE; add keep to supported APIs and mappings; add ASCII log prefix entry for Keep; add Keep tool tiers and update sheets tiers.
Tests & Utilities
tests/gkeep/*, tests/gkeep/test_utils.py, tests/test_scopes.py
Add extensive unit tests for helpers and all Keep tools, test utilities (unwrap, make_note_dict, make_list_note_dict), and scope coverage tests verifying KEEP_WRITE covers KEEP_READONLY.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant MCP_Server as MCP Server
    participant KeepAPI as Google Keep API

    User->>MCP_Server: call create_note(title, text/list)
    MCP_Server->>KeepAPI: POST notes.create (constructed body)
    KeepAPI-->>MCP_Server: 201 Created (note resource)
    MCP_Server-->>User: formatted "Note Created" summary

    User->>MCP_Server: call list_notes(page_size, filter)
    MCP_Server->>KeepAPI: GET notes.list (params)
    KeepAPI-->>MCP_Server: notes + nextPageToken
    MCP_Server-->>User: formatted list (includes nextPageToken if present)

    User->>MCP_Server: call set_permissions(note_id, emails)
    MCP_Server->>KeepAPI: GET note.permissions
    KeepAPI-->>MCP_Server: existing permissions
    MCP_Server->>KeepAPI: batchDelete(non-owner perms)
    MCP_Server->>KeepAPI: batchCreate(new writer perms)
    KeepAPI-->>MCP_Server: operation results
    MCP_Server-->>User: summary of removed/added permissions
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

enhancement

Poem

🐇 I hopped in with scopes on a tiny leap,

Notes and checklists now safely keep.
I stitched a bridge from API to pen,
So thoughts and shopping live easy again. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add Google Keep Integration' clearly and concisely summarizes the main change—adding Google Keep API support with its associated tools.
Description check ✅ Passed The description follows the template with all required sections completed: clear summary of changes, correct type-of-change selection, testing confirmation, and all checklist items checked.
Docstring Coverage ✅ Passed Docstring coverage is 88.57% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

Migrating from UI to YAML configuration.

Use the @coderabbitai configuration command in a PR comment to get a dump of all your UI settings in YAML format. You can then edit this YAML file and upload it to the root of your repository to configure CodeRabbit programmatically.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (12)
core/api_enablement.py (1)

5-5: Minor formatting: missing space before =.

PEP 8 recommends spaces around = in type annotations with default values.

✏️ Suggested fix
-SUPPORTED_APIS: Dict[str, Tuple[List[str], str]]= {
+SUPPORTED_APIS: Dict[str, Tuple[List[str], str]] = {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/api_enablement.py` at line 5, The type annotation for SUPPORTED_APIS is
missing a space before the equals sign; update the declaration of the
SUPPORTED_APIS variable (symbol name SUPPORTED_APIS) so the annotation reads
"Dict[str, Tuple[List[str], str]] = {" (add a space before the '=') to comply
with PEP 8 formatting.
tests/gkeep/test_list_notes.py (2)

74-86: Consider asserting the filter parameter is passed correctly.

The test verifies list was called but doesn't confirm the filter parameter was actually passed to the API. This weakens the test's ability to catch regressions.

🧪 Strengthened assertion
     await unwrap(list_notes)(
         service=mock_service,
         user_google_email="test@example.com",
         filter="trashed=true",
     )

-    mock_service.notes().list.assert_called()
+    # Verify filter was passed through to the API call
+    mock_service.notes().list.assert_called_with(filter="trashed=true")

Alternatively, use call_args to inspect the parameters if other defaults are also passed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/gkeep/test_list_notes.py` around lines 74 - 86, Update the
test_list_notes_passes_filter test so it asserts that the filter parameter is
forwarded to the API: after calling await unwrap(list_notes)(...), inspect
mock_service.notes().list.call_args or
mock_service.notes().list.assert_called_with(...) to verify that the 'filter'
keyword is set to "trashed=true" (while allowing other default args if needed by
using call_args and checking call_args.kwargs['filter']). This ensures the
list_notes function passes the correct filter to mock_service.notes().list().

89-99: Consider verifying the capped page_size value.

The test description states it should "cap page_size to the max" but doesn't assert that the API received the capped value (1000) rather than the requested 5000.

🧪 Strengthened assertion
+from gkeep.keep_tools import LIST_NOTES_PAGE_SIZE_MAX
+
 `@pytest.mark.asyncio`
 async def test_list_notes_caps_page_size():
     """list_notes should cap page_size to the max."""
     mock_service = Mock()
     mock_service.notes().list().execute.return_value = {"notes": []}

     await unwrap(list_notes)(
         service=mock_service,
         user_google_email="test@example.com",
         page_size=5000,
     )
+
+    # Verify pageSize was capped to the maximum
+    call_kwargs = mock_service.notes().list.call_args.kwargs
+    assert call_kwargs.get("pageSize") == LIST_NOTES_PAGE_SIZE_MAX
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/gkeep/test_list_notes.py` around lines 89 - 99, Update
test_list_notes_caps_page_size to assert that the underlying API was called with
the capped page size (1000): after awaiting unwrap(list_notes)(...), inspect the
call args on mock_service.notes().list (e.g., call_args or call_args.kwargs) and
add an assertion that the pageSize kwarg equals 1000 so the test verifies
list_notes actually enforces the cap.
tests/gkeep/test_set_permissions.py (1)

90-122: Consider asserting that batchDelete was not invoked.

The test docstring states it "should skip batchDelete when no non-owner permissions exist" but doesn't verify this behavior. Adding an assertion would strengthen the test.

🧪 Strengthened assertion
     result = await unwrap(set_permissions)(
         service=mock_service,
         user_google_email="test@example.com",
         note_id="notes/abc123",
         emails=["new@example.com"],
     )

+    # Verify batchDelete was NOT called since there were no non-owner permissions
+    mock_service.notes().permissions().batchDelete.assert_not_called()
+
     assert "Removed 0" in result
     assert "Added 1" in result
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/gkeep/test_set_permissions.py` around lines 90 - 122, The test
test_set_permissions_no_existing_writers should assert that batchDelete was not
called: after calling unwrap(set_permissions)(...) add an assertion like
mock_service.notes().permissions().batchDelete.assert_not_called() (or
equivalent) to verify the code path skipped deletion; reference the test
function name, the mock_service mock, and the set_permissions invocation to
locate where to add this assertion.
main.py (1)

259-262: Duplicate icon between Keep and Forms tools.

Both "keep" (line 259) and "forms" (line 262) use the 📝 emoji. Consider using a distinct icon for Keep to improve visual differentiation in CLI output.

🔧 Suggested icon alternatives for Keep
     tool_icons = {
         "gmail": "📧",
         "drive": "📁",
         "calendar": "📅",
         "docs": "📄",
-        "keep": "📝",
+        "keep": "📒",
         "sheets": "📊",
         "chat": "💬",
         "forms": "📝",

Alternative options: 📒 (notebook), 📓 (notebook with decorative cover), or 🗒️ (spiral notepad).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main.py` around lines 259 - 262, The "keep" and "forms" tool entries both use
the same 📝 emoji; update the "keep" mapping to a distinct icon (e.g., replace
the value for the "keep" key with 📒, 📓, or 🗒️) so it no longer duplicates the
"forms" icon and improves CLI visual differentiation; locate the "keep" and
"forms" entries in the tool/icon mapping and change only the "keep" value.
tests/gkeep/test_get_note.py (3)

22-36: Consider verifying the mock was called with expected arguments.

The tests verify output strings but don't assert that the service was called with the correct parameters. Adding mock_service.notes().get.assert_called_with(name="notes/abc123") would strengthen the test coverage.

🧪 Example enhancement
     assert "Test Note" in result
     assert "notes/abc123" in result
     assert "Hello world" in result
+    mock_service.notes().get.assert_called_with(name="notes/abc123")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/gkeep/test_get_note.py` around lines 22 - 36, Add an assertion to
verify the mock service was invoked with the expected parameters: after calling
test_get_note_with_full_name (which awaits unwrap(get_note)), assert that
mock_service.notes().get was called with name="notes/abc123" (use
mock_service.notes().get.assert_called_with(name="notes/abc123")) so the test
not only checks return content but also that get_note invoked the service
correctly.

6-6: Unused import: AsyncMock.

AsyncMock is imported but never used in this module. Consider removing it.

🧹 Proposed fix
-from unittest.mock import AsyncMock, Mock, patch
+from unittest.mock import Mock
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/gkeep/test_get_note.py` at line 6, Remove the unused AsyncMock import
from the test module by editing the import statement "from unittest.mock import
AsyncMock, Mock, patch" to only import the mocks actually used (e.g., "Mock,
patch"); ensure no references to AsyncMock remain in
tests/gkeep/test_get_note.py and run tests to confirm.

39-52: Test for short ID should verify prefix normalization.

This test verifies output but doesn't confirm that the short ID "abc123" was normalized to "notes/abc123" when calling the API. Consider adding an assertion on the mock call.

🧪 Example enhancement
     assert "Test Note" in result
+    # Verify that the short ID was normalized with "notes/" prefix
+    mock_service.notes().get.assert_called_with(name="notes/abc123")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/gkeep/test_get_note.py` around lines 39 - 52, The test
test_get_note_with_short_id currently only checks the returned content but
should also verify the short note_id was normalized to include the "notes/"
prefix when calling the API: add an assertion on the mock_service chain
(mock_service.notes().get) to confirm it was invoked with name="notes/abc123"
(or otherwise that the name argument equals "notes/abc123") before asserting the
result; reference the test function test_get_note_with_short_id, the get_note
wrapper being invoked, and the mock_service.notes().get() call to locate where
to add the assertion.
gkeep/keep_helpers.py (1)

170-176: Minor: Consider using indent: str = "" instead of Optional[str].

Since None is immediately converted to "", using str = "" as the default is clearer.

♻️ Proposed fix
-def _format_list_item(item: ListItem, indent: Optional[str] = None) -> List[str]:
-    indent = indent or ""
+def _format_list_item(item: ListItem, indent: str = "") -> List[str]:
     marker = "x" if item.checked else " "
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gkeep/keep_helpers.py` around lines 170 - 176, Change the _format_list_item
signature to use a concrete default string for indent (e.g., indent: str = "")
instead of Optional[str] so callers and type checkers don't expect None; update
the function body to remove the indent = indent or "" line and keep recursive
calls as _format_list_item(child, indent=f'  {indent}') so behavior is
unchanged, and ensure the type hint for indent in the signature and any
references to it (e.g., other callers) are adjusted accordingly.
core/tool_tiers.yaml (1)

83-83: Remove stale TODO comment.

The comment # TODO: Keep appears to be outdated since this PR implements Keep integration.

🧹 Proposed fix
-# TODO: Keep
 keep:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/tool_tiers.yaml` at line 83, Remove the stale comment "# TODO: Keep"
from core/tool_tiers.yaml since the PR already implements Keep integration;
locate the comment in the tool_tiers.yaml file and delete that line so the file
no longer contains the outdated TODO.
gkeep/keep_tools.py (1)

35-35: Parameter filter shadows Python built-in.

Using filter as a parameter name shadows the built-in filter() function. Consider renaming to filter_query or note_filter.

♻️ Proposed fix
-    filter: Optional[str] = None,
+    filter_query: Optional[str] = None,

And update the usage on line 60:

-        if filter:
-            params["filter"] = filter
+        if filter_query:
+            params["filter"] = filter_query
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gkeep/keep_tools.py` at line 35, Rename the parameter named filter in the
function signature in gkeep/keep_tools.py to a non-shadowing name such as
filter_query (keeping the type Optional[str]) and update every usage inside that
function (including the usage referenced around the current usage on line 60) to
use filter_query; also update the function's docstring, type hints, and any
callers in the module that pass the old parameter name so nothing breaks.
tests/gkeep/test_create_note.py (1)

6-6: Unused imports: AsyncMock and patch.

Neither AsyncMock nor patch is used in this test module.

🧹 Proposed fix
-from unittest.mock import AsyncMock, Mock, patch
+from unittest.mock import Mock
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/gkeep/test_create_note.py` at line 6, The import line in
tests/gkeep/test_create_note.py currently imports AsyncMock and patch which are
unused; update the import to only import Mock (i.e., change "from unittest.mock
import AsyncMock, Mock, patch" to "from unittest.mock import Mock") or remove
the unused names so the module no longer contains unused imports (refer to the
import statement and the symbol Mock to locate the change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@gkeep/keep_tools.py`:
- Around line 417-425: The call to
service.notes().permissions().batchCreate(...).execute is missing parentheses so
the request function isn't being invoked; update the invocation in the
batchCreate block to call .execute() (i.e., change .execute to .execute()) so
that the request is actually executed when awaited inside asyncio.to_thread;
locate the batchCreate call that builds permission_requests in keep_tools.py and
fix the execute invocation on that request object.
- Around line 53-88: The try/except block inside the list_notes function
duplicates error handling already provided by the `@handle_http_errors` decorator;
remove the entire try/except (the HttpError and generic Exception handlers) so
exceptions bubble to the decorator, and adjust indentation of the remaining code
in list_notes (keep references to service.notes().list(...).execute,
Note.from_api, format_note, and logger.info/return logic) so the function body
remains syntactically correct without the redundant handlers.
- Line 14: Replace the incorrect import of Resource from mcp with the Google API
Resource type: remove or change the line importing mcp.Resource and import
googleapiclient.discovery.Resource (or use a type alias like from
googleapiclient.discovery import Resource) so the type of the service parameter
(used in functions that call service.notes(), service.media(), etc.) correctly
refers to googleapiclient.discovery.Resource; update any type hints in
keep_tools.py that reference Resource to use this Google API Resource to prevent
confusion with mcp.Resource.

---

Nitpick comments:
In `@core/api_enablement.py`:
- Line 5: The type annotation for SUPPORTED_APIS is missing a space before the
equals sign; update the declaration of the SUPPORTED_APIS variable (symbol name
SUPPORTED_APIS) so the annotation reads "Dict[str, Tuple[List[str], str]] = {"
(add a space before the '=') to comply with PEP 8 formatting.

In `@core/tool_tiers.yaml`:
- Line 83: Remove the stale comment "# TODO: Keep" from core/tool_tiers.yaml
since the PR already implements Keep integration; locate the comment in the
tool_tiers.yaml file and delete that line so the file no longer contains the
outdated TODO.

In `@gkeep/keep_helpers.py`:
- Around line 170-176: Change the _format_list_item signature to use a concrete
default string for indent (e.g., indent: str = "") instead of Optional[str] so
callers and type checkers don't expect None; update the function body to remove
the indent = indent or "" line and keep recursive calls as
_format_list_item(child, indent=f'  {indent}') so behavior is unchanged, and
ensure the type hint for indent in the signature and any references to it (e.g.,
other callers) are adjusted accordingly.

In `@gkeep/keep_tools.py`:
- Line 35: Rename the parameter named filter in the function signature in
gkeep/keep_tools.py to a non-shadowing name such as filter_query (keeping the
type Optional[str]) and update every usage inside that function (including the
usage referenced around the current usage on line 60) to use filter_query; also
update the function's docstring, type hints, and any callers in the module that
pass the old parameter name so nothing breaks.

In `@main.py`:
- Around line 259-262: The "keep" and "forms" tool entries both use the same 📝
emoji; update the "keep" mapping to a distinct icon (e.g., replace the value for
the "keep" key with 📒, 📓, or 🗒️) so it no longer duplicates the "forms" icon
and improves CLI visual differentiation; locate the "keep" and "forms" entries
in the tool/icon mapping and change only the "keep" value.

In `@tests/gkeep/test_create_note.py`:
- Line 6: The import line in tests/gkeep/test_create_note.py currently imports
AsyncMock and patch which are unused; update the import to only import Mock
(i.e., change "from unittest.mock import AsyncMock, Mock, patch" to "from
unittest.mock import Mock") or remove the unused names so the module no longer
contains unused imports (refer to the import statement and the symbol Mock to
locate the change).

In `@tests/gkeep/test_get_note.py`:
- Around line 22-36: Add an assertion to verify the mock service was invoked
with the expected parameters: after calling test_get_note_with_full_name (which
awaits unwrap(get_note)), assert that mock_service.notes().get was called with
name="notes/abc123" (use
mock_service.notes().get.assert_called_with(name="notes/abc123")) so the test
not only checks return content but also that get_note invoked the service
correctly.
- Line 6: Remove the unused AsyncMock import from the test module by editing the
import statement "from unittest.mock import AsyncMock, Mock, patch" to only
import the mocks actually used (e.g., "Mock, patch"); ensure no references to
AsyncMock remain in tests/gkeep/test_get_note.py and run tests to confirm.
- Around line 39-52: The test test_get_note_with_short_id currently only checks
the returned content but should also verify the short note_id was normalized to
include the "notes/" prefix when calling the API: add an assertion on the
mock_service chain (mock_service.notes().get) to confirm it was invoked with
name="notes/abc123" (or otherwise that the name argument equals "notes/abc123")
before asserting the result; reference the test function
test_get_note_with_short_id, the get_note wrapper being invoked, and the
mock_service.notes().get() call to locate where to add the assertion.

In `@tests/gkeep/test_list_notes.py`:
- Around line 74-86: Update the test_list_notes_passes_filter test so it asserts
that the filter parameter is forwarded to the API: after calling await
unwrap(list_notes)(...), inspect mock_service.notes().list.call_args or
mock_service.notes().list.assert_called_with(...) to verify that the 'filter'
keyword is set to "trashed=true" (while allowing other default args if needed by
using call_args and checking call_args.kwargs['filter']). This ensures the
list_notes function passes the correct filter to mock_service.notes().list().
- Around line 89-99: Update test_list_notes_caps_page_size to assert that the
underlying API was called with the capped page size (1000): after awaiting
unwrap(list_notes)(...), inspect the call args on mock_service.notes().list
(e.g., call_args or call_args.kwargs) and add an assertion that the pageSize
kwarg equals 1000 so the test verifies list_notes actually enforces the cap.

In `@tests/gkeep/test_set_permissions.py`:
- Around line 90-122: The test test_set_permissions_no_existing_writers should
assert that batchDelete was not called: after calling
unwrap(set_permissions)(...) add an assertion like
mock_service.notes().permissions().batchDelete.assert_not_called() (or
equivalent) to verify the code path skipped deletion; reference the test
function name, the mock_service mock, and the set_permissions invocation to
locate where to add this assertion.

Comment thread gkeep/keep_tools.py
Comment thread gkeep/keep_tools.py Outdated
Comment thread gkeep/keep_tools.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@gkeep/keep_tools.py`:
- Around line 277-379: The set_permissions function does not validate
member_type before building permission_requests which can cause a 400 from the
Keep API; add an early guard in set_permissions to check member_type is either
"user" or "group" (e.g., at the top after computing name) and raise/return a
clear error (ValueError or a handled HTTP-style error) if invalid, so
permission_requests is only built with a valid member_type and batchCreate is
never called with an unsupported member_type.

Comment thread gkeep/keep_tools.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/gkeep/test_set_permissions.py`:
- Around line 129-140: The test currently calls
mock_service.notes().permissions().batchDelete.assert_not_called() before
invoking set_permissions, so it can’t detect regressions; move the assertion to
after the await unwrap(set_permissions)(...) call and then assert
mock_service.notes().permissions().batchDelete.assert_not_called() (keeping the
existing checks on the returned result and "Removed 0"/"Added 1") so the test
verifies that batchDelete was not called during set_permissions execution.

Comment thread tests/gkeep/test_set_permissions.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a Google Keep integration to the Google Workspace MCP server, implementing a new gkeep module with tools for reading, creating, listing, deleting notes, downloading attachments, and managing note permissions.

Changes:

  • Adds a new gkeep module with keep_tools.py (7 tools: list_notes, get_note, read_note, create_note, delete_note, download_attachment, set_permissions) and keep_helpers.py (data classes and formatting utilities)
  • Registers the new keep integration across all configuration files: main.py, fastmcp_server.py, manifest.json, smithery.yaml, helm-chart/workspace-mcp/values.yaml, core/tool_tiers.yaml, core/log_formatter.py, core/api_enablement.py, auth/service_decorator.py, and auth/scopes.py
  • Adds comprehensive unit tests for all Keep tools and helper functions in tests/gkeep/

Reviewed changes

Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
gkeep/keep_tools.py Core tool implementations for all 7 Keep operations
gkeep/keep_helpers.py Data classes (Note, ListItem, Attachment, Permission) and formatting helpers
gkeep/__init__.py Module initialization
auth/scopes.py New KEEP_READONLY_SCOPE, KEEP_WRITE_SCOPE constants and scope groups
auth/service_decorator.py Keep service config and scope group registrations
core/api_enablement.py Refactored to support Keep API; adds keep.googleapis.com
core/tool_tiers.yaml Keep tool tier definitions
core/log_formatter.py [KEEP] log prefix for gkeep.keep_tools logger
main.py Registers keep in tool list and imports
fastmcp_server.py Imports Keep tools and adds keep to enabled services
manifest.json Adds google_keep tool entry and keep keyword
smithery.yaml Updates tools description to include keep
helm-chart/workspace-mcp/values.yaml Updates available tools comment to include keep
tests/gkeep/ Comprehensive unit tests for all tools and helpers
tests/test_scopes.py Scope hierarchy test for Keep write/readonly coverage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gkeep/keep_tools.py
Comment thread gkeep/keep_tools.py Outdated
Comment thread gkeep/keep_tools.py Outdated
Comment thread core/tool_tiers.yaml Outdated
hGriff0n added 2 commits March 2, 2026 11:14
The "complete" name threw me off and I thought it was just a combination of the other two sections. Turns out that's not true so we can repurpose 'core' as a "readonly" tier, move the writes to 'extended', and use 'complete' for the rest of the functionality.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@gkeep/keep_helpers.py`:
- Around line 172-183: The build_note_body function currently silently prefers
list_items over text; change it to explicitly reject ambiguous input by adding a
validation at the start of build_note_body that checks if both text and
list_items are provided and raises a ValueError (or appropriate custom
exception) with a clear message; preserve the existing behavior otherwise (use
list_items when only list_items is provided, use text when only text is
provided, and include title always) and reference the build_note_body signature
and the body keys ("body" -> "list" / "text") when making the change.

In `@gkeep/keep_tools.py`:
- Around line 51-52: The log statements currently emit raw PII (email addresses
and collaborator lists) — update the logging in functions that call logger.info
with user_google_email and collaborator/share target lists (e.g., the
[list_notes] invocation and the other occurrences around lines referencing
user_google_email, collaborator lists, or share targets) to avoid raw PII:
replace the raw values with non-identifying info such as masked email (e.g.,
show only domain or initials), hashed/anonymized token, or simple counts (e.g.,
"collaborators_count=3") and include contextual text; search for logger.info
calls that interpolate user_google_email or full lists and change them to log
safe representations instead.
- Around line 54-55: The page_size parameter is only capped at the maximum but
can be zero or negative, causing API errors; update the logic that sets
params["pageSize"] to clamp page_size into a valid positive range by replacing
the current min(...) usage with a clamp to [1, LIST_NOTES_PAGE_SIZE_MAX] (e.g.,
use max(1, min(page_size, LIST_NOTES_PAGE_SIZE_MAX))) or validate and raise a
ValueError for non-positive values so params["pageSize"] is never set to 0 or
negative before calling the Keep API.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c8e14eb and 7f279bf.

📒 Files selected for processing (4)
  • core/tool_tiers.yaml
  • gkeep/keep_helpers.py
  • gkeep/keep_tools.py
  • tests/gkeep/test_download_attachment.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/gkeep/test_download_attachment.py

Comment thread gkeep/keep_helpers.py
Comment thread gkeep/keep_tools.py Outdated
Comment thread gkeep/keep_tools.py Outdated

@fruch fruch left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

exactly what I'm looking for

@fruch

fruch commented Mar 19, 2026

Copy link
Copy Markdown

Bug: Syntax error in keep_tools.py:37 prevents server startup

When running workspace-mcp --tools calendar keep, the server crashes on import with:

SyntaxError: invalid syntax
  File ".../gkeep/keep_tools.py", line 37
    _redact_email(user_google_email): str,
                 ^

The list_notes function signature uses a function call as a parameter name:

async def list_notes(
    service: Resource,
    _redact_email(user_google_email): str,  # <-- invalid syntax
    ...

This should likely be a regular parameter name (e.g. user_google_email: str).

Tested with commit 3985e57 on the keep branch, Python 3.13.

A bit too aggressive on the find-replace
@fruch

fruch commented Mar 19, 2026

Copy link
Copy Markdown

I can confirm it was working up to premission error I need to fight with IT :)

I'll keep updating

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.

3 participants