chore(grammarly): migrate to autohive-integrations-sdk 2.0.0#361
chore(grammarly): migrate to autohive-integrations-sdk 2.0.0#361Shubhank-Jonnada wants to merge 13 commits into
Conversation
- Bump SDK to ~=2.0.0 in requirements.txt (drop aiohttp dependency) - Bump config.json version to 2.0.0; remove result/error from output schemas - Fix auth: flat context.auth.get() instead of nested credentials dict - Replace aiohttp with context.fetch() for all API calls - Error returns now use ActionError instead of ActionResult with result:False - Replace context.py + test_grammarly.py with conftest.py + unit + integration tests - All 7 actions covered by 13 unit tests - Add Grammarly env vars to root .env.example
🔍 Integration Validation ResultsCommit: Changed directories:
✅ Structure Check output✅ Code Check output✅ Tests Check output✅ README Check output✅ Version Check output |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75d5d8dce9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
All unit tests performed and passing. Live integration tests not yet run — Grammarly OAuth credentials required. |
TheRealAgentK
left a comment
TheRealAgentK
left a comment
There was a problem hiding this comment.
Requesting changes for the Grammarly SDK 2 blockers.
|
Before approval or merge, we need a real integration test run with valid credentials, using the SDK 2 request path. The live tests must fail on |
TheRealAgentK
left a comment
TheRealAgentK
left a comment
There was a problem hiding this comment.
Added a coverage follow-up.
SDK 2 ExecutionContext.fetch() accepts data= and content_type=, not body=. The token request (form-encoded) and file upload (text/plain) were both passing body= which would raise TypeError at runtime. Also read client_id/client_secret from context.auth['credentials'] first (production SDK 2 shape) with a flat fallback for unit-test mocks.
MagicMock(data=...) doesn't match the SDK 2 fetch() contract. Replace all
mock responses with FetchResponse(status=200, headers={}, data=...) so
unit tests exercise the same response shape the real SDK returns.
…verage - Add pytest.mark.integration to pytestmark so -m integration selects them - Replace stub _Ctx with aiohttp-backed real_fetch via make_context fixture, matching the SDK 2 fetch() contract (data=, content_type=, HTTPError/RateLimitError) - Assert result.type == ResultType.ACTION for all live-path tests - Add coverage for get_writing_score_results, analyze_ai_detection, get_ai_detection_results, analyze_plagiarism_detection, get_plagiarism_detection_results (all previously missing)
…rmalize dash style
|
Live integration tests passed - 7/7, 19-06-2026. One fix applied during testing: All actions verified against the live API:
|
There was a problem hiding this comment.
Posted two inline review comments from the follow-up review.
After these are addressed, please also add the output of a new integration test run, @Shubhank-Jonnada
| # ---- Writing Score API Actions ---- | ||
| async def upload_file(context: ExecutionContext, upload_url: str, file_content: str) -> bool: | ||
| await context.fetch( | ||
| upload_url, |
There was a problem hiding this comment.
Blocker: this upload path now passes the pre-signed URL to context.fetch() as a plain string, which loses the old yarl.URL(upload_url, encoded=True) safeguard. These upload URLs are S3-style signed URLs, and aiohttp can canonicalize encoded query parameters (for example %2F in X-Amz-Credential) when it builds the request URL, invalidating the signature. Please preserve the encoded URL here, e.g. by passing URL(upload_url, encoded=True) through to context.fetch() if supported, or by keeping a dedicated aiohttp upload helper that uses encoded=True. A small unit test around the upload URL/call shape would help prevent this from regressing again.
There was a problem hiding this comment.
A minimal version of the fix should be possible by passing a yarl.URL object through to context.fetch() (aiohttp accepts this even though the SDK annotation is str):
from yarl import URL
async def upload_file(context: ExecutionContext, upload_url: str, file_content: str) -> bool:
await context.fetch(
URL(upload_url, encoded=True),
method="PUT",
data=file_content,
content_type="text/plain",
)
return TrueIf we import yarl directly, please add it to grammarly/requirements.txt rather than relying on aiohttp/SDK transitive dependencies. If passing a non-str into context.fetch() is considered too loose, the fallback is to keep a dedicated aiohttp upload helper that does session.put(URL(upload_url, encoded=True), ...) for this one signed-URL upload path.
There was a problem hiding this comment.
Fixed in b78f8de . upload_file now wraps the pre-signed URL in URL(upload_url, encoded=True) from yarl to prevent aiohttp from re-encoding query parameters and invalidating the S3 signature. Added yarl>=1.9 to requirements.txt.
| async def test_get_user_analytics(live_context): | ||
| result = await grammarly.execute_action( | ||
| "get_user_analytics", | ||
| {"date_from": "2025-12-01", "date_to": "2025-12-31"}, |
There was a problem hiding this comment.
Should fix: this hard-coded analytics date range will eventually age out of Grammarly's “past 365 days with 2-day lag” API window, making the live test fail over time. Please compute this dynamically, e.g. date_to = today - 2 days and date_from = date_to - 30 days, then format both as YYYY-MM-DD.
There was a problem hiding this comment.
Fixed in a2d2e71. The analytics test now computes date_to as today minus 2 days and date_from as date_to minus 30 days, formatted as YYYY-MM-DD, so the range stays inside Grammarly's 365-day window indefinitely.
… signed URL integrity S3 pre-signed URLs contain percent-encoded query params (e.g. %2F in X-Amz-Credential) that aiohttp can re-encode when building the request URL, invalidating the signature. Wrapping in URL(upload_url, encoded=True) tells aiohttp the URL is already encoded. Added yarl>=1.9 to requirements.txt rather than relying on it as a transitive dependency.
…on test Hard-coded dates age out of Grammarly's 365-day window with 2-day lag. Changed to date_to = today - 2 days, date_from = date_to - 30 days, formatted as YYYY-MM-DD, so the test stays valid indefinitely.
2 participants
Summary
Test plan