Repository navigation
feat: connect Notion tasks to Astra - #732
Conversation
There was a problem hiding this comment.
Important
The new Notion MCP reader is missing from the running server's tool registry, so MCP clients cannot discover or invoke it.
Reviewed changes
Reviewed the complete Notion integration and its included Astra choice/pick-list prerequisite, including persistence, authorization, provider contracts, and client compatibility.
- Connection lifecycle: Pro-gated OAuth authorization, single-use states, encrypted credentials, connection status, and disconnect cleanup.
- Task snapshots: Scheduled polling, pagination and throttling, user-timezone property mapping, bounded retention, and reconnect/error handling.
- Astra and MCP: Negotiated interactive blocks, one-shot answers, stored-selection bulk previews, Notion tool/catalog wiring, and untrusted-data prompt guidance.
- Persistence and contracts: Migrations, reset/cascade deletion, credential-free exports, append-only HTTP contracts, and the API-first client rollout.
- Verification: The solution build succeeded and all 9,720 tests passed; a separate probe of the production MCP bootstrap reproduced the missing registration. Authenticated Notion staging behavior was not exercised.
gpt-6.1-sol | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed bd70ca7c, the single commit since the prior Pullfrog review, against the full PR context and resolved discussion.
- Registered the Notion reader: Added
NotionToolsto the production MCP bootstrap, addressing the previously reported discovery and invocation failure. - Added runtime registry coverage: Checked every catalog-declared MCP tool against the actual production tool collection rather than relying only on catalog strings or direct wrapper calls.
- Verified the affected paths: All 351 selected MCP, catalog, and Notion tests passed, including the new production-bootstrap regression test. The resolved finding is fixed, and no new blocking issues remain.
gpt-6.1-sol | 𝕏
The squash of orbit-api#731 replaced the commits this branch was stacked on. Five add/add conflicts take the merged #731 version (this branch did not change them beyond its older #731 base); the model snapshot keeps this branch's Notion entities on top of it. The result equals the tree of combined check a2 (base c8877ee with #731, #732 and #730 merged), which passed build, the openapi freshness check and all four test projects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
@pullfrog review |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Read the complete 67-file diff and traced the Notion connection through authorization, persistence, scheduled polling, and its chat and MCP consumers.
- Connection lifecycle: Reviewed the Pro and feature-flag gates, redirect allowlist, single-use OAuth state, encrypted credentials, and reset/disconnect handling for in-flight operations.
- Task snapshots: Reviewed provider requests and pagination against current Notion documentation, per-token throttling, user-local date mapping, account-wide retention cap, and transactional snapshot replacement.
- Agent surfaces: Verified the shared read tool, capability and scope mappings, production MCP registration, and untrusted-data prompt framing.
- Data compatibility: Checked migrations, cascade cleanup, credential-free exports, and the sibling consumer contracts. Existing contracts remain additive; the client connection UI is explicitly deferred to the blocked follow-up ticket.
- Verification: All 91 focused infrastructure, application, and domain tests passed, including production MCP registration coverage. Authenticated Notion staging was not exercised; the documented staging validation remains necessary before enabling the production flag.
gpt-6.1-sol | 𝕏




Closes thomasluizon/orbit-tickets#1484.
Depends on #731, which carries thomasluizon/orbit-tickets#1480. Merge that dependency before this PR. This API change blocks thomasluizon/orbit-tickets#1485.
Approach
Pro users can authorize a public Notion connection, complete the server-side exchange, read connection status and shared databases, and disconnect through
ConnectionsController. Commands and validators live inOrbit.Application/Connections; Notion entities and generated migrations persist encrypted credentials, a bounded task snapshot and single-use authorization states. Consumed states remain present until exchange completion so reset or disconnect can invalidate an exchange already in flight.Orbit.Infrastructure/Services/Notionowns the documented REST interface, per-token request queue, fullRetry-Afterwaits, property mapping and scheduled poll. The poll runs every 15 minutes in both scheduler modes, claims its window atomically, keeps at most 100 tasks across sources, and checks the connection again before saving. A 401 requires reconnection; transient failures preserve the previous snapshot. Pro access andnotion_integrationgate every endpoint, poll and stored-task read. Notion forces a fresh flag read so a warm cache cannot delay the kill switch.The keyword-gated
get_notion_taskstool, its capability registration and prompt guidance feed Notion data into the existingaskpick list withcreate_habits, then the existing preview. A small MCP wrapper delegates to the same reader to satisfy the repository's capability parity contract. Task text is explicitly framed as untrusted data and titles are capped. Reset, cascade deletion and the append-only export cover the new data; exports never contain credentials.src/Orbit.Api/openapi.jsonis regenerated from the controllers and contracts.Test evidence
dotnet build Orbit.slnx: 0 errors.dotnet test tests/Orbit.Domain.Tests --no-build --no-restore(678 passed),dotnet test tests/Orbit.Application.Tests --no-build --no-restore(5,608 passed),dotnet test tests/Orbit.Infrastructure.Tests --no-build --no-restore(3,402 passed), anddotnet test tests/Orbit.Analyzers.Tests --no-build --no-restore(32 passed). Total: 9,720 passed, none skipped. Verification completed within the 25-minute budget.dotnet test tests/Orbit.Infrastructure.Tests --no-restore --filter 'FullyQualifiedName~Notion|FullyQualifiedName~ConnectionsControllerTests'initially passed 38 cases and failed the separate trial eligibility case. The later focused set passed 45 cases before the reset/exchange test was added.dotnet test tests/Orbit.Infrastructure.Tests --no-restore --filter FullyQualifiedName~ResetOrDisconnectInvalidatesAnExchangeAlreadyInFlight: both reset and disconnect cases failed because the callback incorrectly returned 204 and recreated the connection. Restored the consumed-state fix and reran the focused infrastructure set: all 75 passed, including both cases.Retry-Afterwait without a 60-second cap, bounded retries, documented auth, pagination and revocation. SQLite persistence tests inspect ciphertext columns, foreign-key cascades and reset cleanup without a live database. Fixtures and their documentation provenance are intests/Orbit.Infrastructure.Tests/Fixtures/Notion.PollPersistsWhenProviderMaterializesTimestampsAtMicrosecondPrecision, using a materialization interceptor to reproduce that documented precision without a live database.dotnet test tests/Orbit.Infrastructure.Tests --no-restore --filter FullyQualifiedName~PollPersistsWhenProviderMaterializesTimestampsAtMicrosecondPrecisionfailed with zero saved tasks instead of one before the fix. Moving claim equality into the database query made it pass; the focused set then passed all 76 cases.CapabilityParityTests.EveryCapability_HasMatchingChatAndMcpSurfacestest because the Notion reader lacked an MCP mapping. Added a wrapper over the shared read tool and two behavior tests. The focused run including that parity test passed all 79 cases.5fc3a739, unchanged tests passed:dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~CapabilityParityTests|FullyQualifiedName~NotionToolsTests'Exit 0, 3 passed. After the fix: exit 0, 4 passed.
get_notion_taskswas unregistered:dotnet test tests/Orbit.Infrastructure.Tests --filter FullyQualifiedName~EveryDeclaredMcpTool_IsRegisteredByProductionBootstrapExit 1. It passed after registration.
dotnet build Orbit.slnx: exit 0, zero errors. OpenAPI regenerated without changes.dotnet test tests/Orbit.Domain.Tests: exit 0, 678 passed.dotnet test tests/Orbit.Application.Tests: exit 0, 5,608 passed.dotnet test tests/Orbit.Infrastructure.Tests: exit 0, 3,403 passed.MCP 1.4.1 fields verified against its exact package-source revision: ToolCollection, ProtocolTool, Name.
Notion interface evidence
All pages below were read live on 2026-10-17 before writing the corresponding client behavior. Re-derive the evidence by fetching each linked page with
.mdappended; endpoint pages contain the current OpenAPI schema. Credentials do not exist yet, so no authenticated Notion response or staging flow is claimed.https://api.notion.com/v1/oauth/authorize;owner=user,response_type=code,client_id,redirect_uri,state; page picker and code return; server-side Basic authentication.POST /v1/oauth/token; authorization-code grant,code, matching redirect URI; responseaccess_token, nullablerefresh_token,workspace_id, nullableworkspace_name,bot_id. Other response fields are ignored.POST /v1/oauth/revoke, Basic authentication and{token}; success depends on the HTTP status. No response field is required by the implementation.POST /v1/search, filter{property:"object",value:"data_source"}and no query to discover shared sources, including their property schemas.id,parent.database_id, rich-texttitle,properties; data sources belong to database containers.GET /v1/databases/{id}and rich-texttitleprovide the actual database name rather than a child source's name.POST /v1/data_sources/{id}/query,result_type:"page", paginated results and the documented 10,000-result query ceiling. The account cap stops retention at 100; no completeness claim is made beyond the provider's query ceiling.page_sizemaximum 100,start_cursor,results,has_more,next_cursor; cursors are opaque.id,properties,url,last_edited_time; recorded page envelope used by the mapping tests.idandtype; title rich-textplain_text; nullable datestartandtime_zone; statusidandname; schema status groupsnameandoption_ids; checkbox boolean. Open membership comes from the documented To-do/In progress groups, including custom option names.Retry-After, retry with backoff, and stop after three retries. Documentedadditional_data.rate_limit_reason=public_api_request_blockedstops retries.Notion-Version: 2026-03-11; bearer auth for data calls; HTTP status drives reconnect/transient behavior.Assumptions
redirectUriin its request body and binds it to state; rejected a single hard-coded redirect because configuration allows multiple clients.AiToolRegistry.CapabilityParityTests; rejected weakening the parity guard or duplicating Notion read logic.Manual steps
Notion__ClientIdandNotion__ClientSecretfrom that connection's Configuration tab. SetNotion__AllowedRedirectUris__0and subsequent indexed keys to exactly the same approved return URLs. Confirm the existingEncryption__Keyis configured. Prove the settings through successful authorization with an allowed URI and rejection of an unlisted URI. Repeat the settings on the production API service only for rollout.AppFeatureFlagstable, enablenotion_integrationonly for the staging flow after configuration is present; confirmPlanRequirementremainsPro. Verify polling, property mapping, reconnect handling, the existing pick list preview and disconnect cleanup. In the production table keepEnabled=falseuntil the staging flow, #1485 and any Notion review are complete. Verify a disabled flag returns the existing gate error and stops new reads and polls. No flag or environment was changed by this worker.redesign/mainlistsget_notion_tasksin the authenticated MCPtools/list. No new configuration or secrets are required.Eval scenarios for #26
get_notion_tasks, answers only from its result and identifies the stored snapshot's poll time. If offering conversion, use the existingcreate_habitspick list and preview for the chosen items.