fix(activity): viewer-form GraphQL + token-scope guidance#25
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughDocumentation and error handling improvements clarifying GitHub token scope requirements and their effects on contribution visibility. The code now throws descriptive errors when viewer data is missing during GraphQL pagination instead of silently breaking. UI and setup documentation updated to explain token scope limitations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 2/5 reviews remaining, refill in 24 minutes and 56 seconds. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 43-53: Update the README section titled "Pick a token type" to
stop calling activity scope "allowlist-based" and instead clarify that "Sync
activity" returns contributions only from repositories visible to the token;
specifically, revise the phrasing in the "Fine-grained PAT (recommended for
repo-scoped syncs)" and "Classic PAT (simplest path for `Sync activity`)"
bullets to say "all repos visible to the token" vs "narrower token scope" (and
remove language implying the plugin is filtered by a separate allowlist), ensure
the sentence “Repository access matters for `Sync activity`” explicitly explains
that setting Repository access to "Only select repositories" limits
contributions to repos the token can see, and update any mention of "allowlisted
repos" to "repos visible to the token" to avoid implying a separate allowlist
filters syncs.
In `@src/github/graphql.ts`:
- Around line 449-455: The paginated fetch code in fetchContributionsCollection
uses "if (!data.viewer) break" which silently truncates results; instead, detect
a null viewer from the client.graphql call and surface the failure the same way
the main query branch does (e.g., log an error and throw or return an explicit
error) so partial results aren't returned silently; update the blocks around the
client.graphql<PaginatePullRequestsResponse>(...) call (and the analogous blocks
for the other paginated queries) to check for data.viewer and handle it by
emitting a consistent error (or throwing) rather than breaking out of the loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fdfd152e-f996-4fdb-ae8c-bda76bc55e8a
📒 Files selected for processing (9)
README.mdmanifest.jsonpackage.jsonsrc/github/graphql.test.tssrc/github/graphql.tssrc/settings/settings-tab.tssrc/sync/activity-writer.test.tssrc/sync/activity-writer.tsversions.json
📜 Review details
🔇 Additional comments (4)
src/github/graphql.test.ts (4)
35-66:mainResponsefixture is correctly aligned to the new GraphQL shape.Switching the mock root to
viewer.contributionsCollectionis consistent with the production query contract and keeps downstream test data coherent.
112-132: Variable and query-shape assertions are solid here.This test now correctly locks in both migration guarantees: no
loginvariable andviewer-based query text.
134-143: Failure-path expectation update looks correct.The new rejection assertion matches the updated behavior when
viewercontributions are missing.
210-218: Pagination mocks were updated consistently across all connection branches.Using
viewerin all paginated mock payloads avoids shape drift and keeps pagination tests representative of runtime responses.Also applies to: 248-256, 260-268, 272-280, 328-336
- README: drop the "allowlisted repos" framing for `Sync activity`. The activity command is user-centric and is not filtered by the allowlist; the actual switch is "all repos visible to the token" vs a narrower scope. Adds a parenthetical clarifying which commands the allowlist gates. - graphql.ts: throw instead of `break` when a paginated `viewer` fetch returns null. The silent-break path could swallow auth/shape failures partway through pagination and return a partial activity set without surfacing the error -- exactly the silent-undercount class this PR was opened to fix. Behavior now matches the main-query branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`viewer.contributionsCollection` only returns contributions to repos the token can see, so a fine-grained PAT scoped to "Only select repositories" silently drops every commit/PR/issue/review on unscoped repos -- the activity sync succeeds with 0 day files. Splits the README Setup token step into fine-grained vs classic guidance and adds a callout above the Activity window setting so users hit the warning next to the control they're configuring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- README: drop the "allowlisted repos" framing for `Sync activity`. The activity command is user-centric and is not filtered by the allowlist; the actual switch is "all repos visible to the token" vs a narrower scope. Adds a parenthetical clarifying which commands the allowlist gates. - graphql.ts: throw instead of `break` when a paginated `viewer` fetch returns null. The silent-break path could swallow auth/shape failures partway through pagination and return a partial activity set without surfacing the error -- exactly the silent-undercount class this PR was opened to fix. Behavior now matches the main-query branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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 `@README.md`:
- Around line 43-54: The settings UI copy in src/settings/settings-tab.ts
currently instructs users to create only fine-grained PATs; update the
token-field help text (the string/constant used by the SettingsTab component or
the function renderTokenHelp / tokenHelpText) to match the README: explain that
both fine-grained and classic PATs are supported, include both links
(github.com/settings/personal-access-tokens and github.com/settings/tokens),
list the recommended read-only scopes for fine-grained PATs (Contents: read,
Issues: read, Pull requests: read, Metadata: read, Dependabot alerts: read,
Actions: read) and note that classic PATs should include read:user (and repo
only if private contributions need counting). Ensure the copy also preserves the
Repository access note about "Only select repositories" vs "All repositories"
for Sync activity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d5a61ac-f43e-473a-966f-e11c780e1ef8
📒 Files selected for processing (2)
README.mdsrc/github/graphql.ts
📜 Review details
🔇 Additional comments (1)
src/github/graphql.ts (1)
369-375: Good hardening on null-viewerhandling in both main and paginated paths.The new throw behavior prevents silent partial activity results and aligns failure surfacing across all contribution fetch branches.
Also applies to: 453-458, 487-493, 521-528
54092fa to
7d89c75
Compare
* [github-data] docs: flag token-scope requirement for Sync activity `viewer.contributionsCollection` only returns contributions to repos the token can see, so a fine-grained PAT scoped to "Only select repositories" silently drops every commit/PR/issue/review on unscoped repos -- the activity sync succeeds with 0 day files. Splits the README Setup token step into fine-grained vs classic guidance and adds a callout above the Activity window setting so users hit the warning next to the control they're configuring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [github-data] fix(activity): address review feedback on PR #25 - README: drop the "allowlisted repos" framing for `Sync activity`. The activity command is user-centric and is not filtered by the allowlist; the actual switch is "all repos visible to the token" vs a narrower scope. Adds a parenthetical clarifying which commands the allowlist gates. - graphql.ts: throw instead of `break` when a paginated `viewer` fetch returns null. The silent-break path could swallow auth/shape failures partway through pagination and return a partial activity set without surfacing the error -- exactly the silent-undercount class this PR was opened to fix. Behavior now matches the main-query branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [github-data] docs: add public ROADMAP.md mirroring the internal phase ladder Public phase ladder (v0.1 -> v1.0) covering shipped surface, near-term v0.2 close-out (cron polling + SOP draft), v0.3 candidates (Telemetry Grid feed, Heatmap Calendar, charter hydration), out-of-scope items, and the known fine-grained-PAT gotcha for `Sync activity`. Internal design doc in the vault stays authoritative; this is the contributor / reviewer mirror. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
contributionsCollectionqueries fromuser(login: ...)toviewer { ... }. Theuserform is treated as a third-party query and silently drops granular private-repo data even with the public-profile toggle on;viewerreturns the authenticated user's complete contribution graph.viewer.contributionsCollectiononly returns contributions to repos the token can see, so a fine-grained PAT scoped to "Only select repositories" silently drops every commit / PR / issue / review on unscoped repos and produces 0 day files. README Setup now splits fine-grained vs classic guidance, and the settings tab shows a callout above the Activity window control.Test plan
npm test); GraphQL + activity-writer tests updated for the new shape.npm run typecheckclean.read:user+repoproduces non-zero day files for the activity sync (Cap, 2026-04-30).🤖 Generated with Claude Code