fix(memory/sync): send fetch_type in Notion NOTION_FETCH_DATA args - #144
fix(memory/sync): send fetch_type in Notion NOTION_FETCH_DATA args#144YellowSnnowmann wants to merge 1 commit into
Conversation
Composio's NOTION_FETCH_DATA now requires fetch_type; the memory-sync Notion provider's arguments() omitted it, so every periodic sync 400s. Closes #5506
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe Notion memory sync request now includes ChangesNotion memory sync
Estimated code review effort: 1 (Trivial) | ~5 minutes Mergeability Score: ⚪ Minimal · up to The Notion memory-sync request now includes the required page fetch type, restoring compatibility without changing public APIs, permissions, data scope, or deployment behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
What this change touches2 files, +4 -1 across 2 components. The code graph knows nothing about these files yet — normal for newly added files, and a cold index otherwise. flowchart LR
n0["tests<br/>1 file +3 -0"]:::changed
n1["src/memory/sync/composio/providers<br/>1 file +1 -1"]:::changed
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.
Changed files
|
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0077 · 10,747 in / 2,186 out · 9,038 cached (84%) · z-ai/glm-5.2
critique: $0.0040 · 3,138 in / 1,395 out · 2,765 cached (88%) · z-ai/glm-5.2
security: $0.0011 · 3,096 in / 203 out · 2,729 cached (88%) · z-ai/glm-5.2
tests: $0.0010 · 1,664 in / 215 out · 1,222 cached (73%) · z-ai/glm-5.2
description: $0.0006 · 2,004 in / 82 out · 1,726 cached (86%) · z-ai/glm-5.2
Summary
Composio's
NOTION_FETCH_DATAaction now requires afetch_typefield. The memory-sync Notion provider'sarguments()never sent it, so every periodic Notion memory sync failed withFollowing fields are missing: {'fetch_type'}.This adds
"fetch_type": "pages"to the sync provider's request args. It's unconditional because this path hardcodesfilter: {value: "page"}, so"pages"is the only valid value (the agent-tool path's inference logic is deliberately not duplicated here).The agent-tool path in OpenHuman already handles this via
ensure_notion_fetch_type; only this memory-sync path was still missing the field.Fixes tinyhumansai/openhuman#5506.
API Or Behavior Changes
None — internal request-args shape only. Adds a field the upstream Composio API now requires.
Tests
cargo fmt --checkcargo clippy --all-targets -- -D warningscargo build --all-targetscargo test— 1396 tests, 0 failedAdded a
body_partial_jsonmatcher on theNOTION_FETCH_DATAmock intests/composio_sync_mock.rsasserting the outgoing request carriesfetch_type: "pages"(fails pre-fix), following the existing Slack/ClickUp request-body matcher pattern in the same file.Documentation
None needed — internal provider request shape.
Summary by CodeRabbit