Conversation
- Add MY_OPEN_PRS and MY_PR_STATUS dynamic providers (myPrStatus.ts) - Add GITHUB_PR_STATUS_ORG_FILTER for org-scoped PR status - Add Data Enricher registry on GitHubService (registerEnricher, enrichItems) - Register built-in pr-merge-review-status enricher for github:pull_request - Add MyOpenPrItem, DataEnricher types; state-helpers, serialize utils - Remove provider/utils index barrels; pullRequests imports from concrete utils - PR URL regex accepts /pull/ and /pulls/; use getPullRequestFromState/getRepositoryFromState - .gitignore: .turbo/, *.tsbuildinfo; README and CHANGELOG updates - Tests: enricher-registry.test, myPrStatus.test Made-with: Cursor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (24)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| changesRequested, | ||
| clearToMerge: | ||
| fullPr.mergeable === true && !changesRequested, | ||
| }; |
There was a problem hiding this comment.
Enricher assumes MyOpenPrItem shape, fails on raw PRs
High Severity
The built-in pr-merge-review-status enricher casts every item to MyOpenPrItem and reads top-level pr.owner, pr.repo, and pr.number. This works for MY_PR_STATUS which passes normalized MyOpenPrItem objects. However, LIST_GITHUB_PULL_REQUESTS calls enrichItems("github:pull_request", prs) with raw Octokit API objects, which have base.repo.owner.login and base.repo.name instead of top-level owner/repo. The enricher calls getPullRequest(undefined, undefined, ...), silently throws, and enrichment is swallowed by the catch — so list PR enrichment always silently produces no data.
Additional Locations (1)
| } | ||
|
|
||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Multiple new exported functions are never imported
Low Severity
Several newly exported functions are never imported or used anywhere in the codebase: getIssueFromState in state-helpers.ts, and sanitizeComment, sanitizeArray, and sanitizeGitHubObject in serialize.ts. Grep confirms each only appears at its definition site. This is dead code that adds maintenance burden.
Additional Locations (2)
There was a problem hiding this comment.
Pull request overview
This PR adds “My PR Status” dynamic providers and a GitHubService enricher registry to attach per-item metadata (e.g., merge/review readiness) to list results, while also improving state handling, serialization safety, and removing provider barrel exports.
Changes:
- Added
MY_OPEN_PRSandMY_PR_STATUSdynamic providers backed byGitHubService.getMyOpenPRs()plus optional enrichment. - Introduced an enricher registry on
GitHubService(registerEnricher,enrichItems) and registered a built-in PR merge/review status enricher. - Added state helper utilities and serialization utilities; updated multiple actions to use enrichment and improved state fallback handling.
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/state-persistence.ts | Deduplicates concurrent persisted-state loads via an in-flight promise cache. |
| src/utils/state-helpers.ts | Adds helpers for reading/writing GitHub context consistently from state (with legacy fallback). |
| src/utils/serialize.ts | Introduces sanitizers to avoid circular references when returning/storing GitHub API objects. |
| src/types.ts | Adds DataEnricher and MyOpenPrItem types to support the enricher registry and PR status providers. |
| src/services/github.ts | Adds enricher registry + enrichItems, updates repo listing params, and adds getMyOpenPRs. |
| src/providers/myPrStatus.ts | Implements MY_OPEN_PRS and MY_PR_STATUS dynamic providers (with optional org filter + enrichment). |
| src/providers/index.ts | Removes provider barrel exports. |
| src/providers/github.ts | Minor formatting fix in exports list. |
| src/index.ts | Registers new providers and registers the built-in PR merge/review status enricher during plugin init. |
| src/actions/webhooks.ts | Adds legacy state.github fallback for repository context. |
| src/actions/users.ts | Tightens validators and sanitizes repository objects returned in callback payloads. |
| src/actions/stats.ts | Tightens validators and prefers state.data.github repository context where available. |
| src/actions/search.ts | Tightens validation/query extraction logic and adds logging around query extraction. |
| src/actions/repository.ts | Improves owner/repo parsing, adds optional enrichment, and sanitizes repositories in callback payloads. |
| src/actions/pullRequests.ts | Expands PR URL matching, adds state helper usage, optional enrichment, and PR sanitization for callback payloads. |
| src/actions/issues.ts | Improves owner/repo parsing + state fallback, adds optional enrichment, and sanitizes issues in search callback payloads. |
| src/actions/branches.ts | Tightens validators and adds optional enrichment for branch list results. |
| src/actions/activity.ts | Tightens validators for activity-related actions. |
| src/tests/myPrStatus.test.ts | Adds tests for getMyOpenPRs, the two providers, and JSON-serializable enricher output. |
| src/tests/enricher-registry.test.ts | Adds tests covering enricher registry behavior, concurrency, and non-mutation. |
| package.json | Documents GITHUB_PR_STATUS_ORG_FILTER in the plugin settings schema. |
| README.md | Documents new providers, org filter, and the enricher registry concept/usage. |
| CHANGELOG.md | Adds an “Unreleased” changelog entry describing new features and behavior changes. |
| .gitignore | Adds .turbo/ and *.tsbuildinfo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } catch (err) { | ||
| logger.warn( | ||
| { enricher: enricher.name, dataType, item: merged?.number ?? merged?.id }, |
There was a problem hiding this comment.
The catch here logs that an enricher failed but drops the actual error (err), which makes diagnosing failing enrichers difficult. Consider including the error in the structured log payload (e.g., { err, enricher: ..., ... }) so failures can be debugged without reproducing locally.
| { enricher: enricher.name, dataType, item: merged?.number ?? merged?.id }, | |
| { err, enricher: enricher.name, dataType, item: merged?.number ?? merged?.id }, |
| if (!query) { | ||
| throw new Error("Search query is required. Try: 'search for [topic]' or 'find [topic] on github'"); | ||
| logger.warn(`Failed to extract search query from message: "${text}"`); | ||
|
|
||
| // Provide helpful error with context | ||
| const errorMsg = text.length > 0 | ||
| ? `I couldn't extract a search query from your message. Please try:\n- "search for machine learning"\n- "find React repositories"\n- "look up TypeScript issues"\n\nYour message: "${text.substring(0, 100)}${text.length > 100 ? '...' : ''}"` | ||
| : "Search query is required. Try: 'search for [topic]' or 'find [topic] on github'"; |
There was a problem hiding this comment.
Avoid logging raw user message text at warn level here. User messages can contain sensitive data; consider logging only a truncated/hashed value or moving this to debug with a short prefix (e.g., first N chars) and metadata (roomId/messageId) instead.
| // Clean up the query | ||
| query = query.trim().replace(/\s+/g, " "); | ||
|
|
||
| logger.info(`Extracted search query: "${query}" from message: "${text}"`); |
There was a problem hiding this comment.
This info log includes the full original user message. Even at info level this can leak PII/secrets into logs; consider logging only the extracted query (and maybe message length) rather than the entire message text.
| logger.info(`Extracted search query: "${query}" from message: "${text}"`); | |
| logger.info( | |
| `Extracted search query: "${query}" (original message length: ${text.length} characters)`, | |
| ); |
| const responseContent: Content = { | ||
| text: `${issueState.charAt(0).toUpperCase() + issueState.slice(1)} issues for ${owner}/${repo} (${issues.length} shown):\n${issueList}`, | ||
| text: `${issueState.charAt(0).toUpperCase() + issueState.slice(1)} issues for ${owner}/${repo} (${issuesToUse.length} shown):\n${issueList}`, | ||
| actions: ["LIST_GITHUB_ISSUES"], | ||
| source: message.content.source, | ||
| // Include data for callbacks | ||
| issues, | ||
| issues: issuesToUse, | ||
| repository: `${owner}/${repo}`, | ||
| issueCount: issues.length, | ||
| issueCount: issuesToUse.length, | ||
| }; |
There was a problem hiding this comment.
sanitizeIssue is imported but LIST_GITHUB_ISSUES still returns issues: issuesToUse in the callback payload. If circular refs are a concern (as in SEARCH_GITHUB_ISSUES), this should likely use issuesToUse.map(sanitizeIssue) for consistency; otherwise the unused import should be removed.
| // Objects | ||
| const sanitized: any = {}; | ||
| for (const key in value) { | ||
| if (value.hasOwnProperty(key)) { |
There was a problem hiding this comment.
sanitizeGitHubObject uses value.hasOwnProperty(key), which can be unsafe if hasOwnProperty is shadowed and also misses edge cases like __proto__ keys. Prefer Object.prototype.hasOwnProperty.call(value, key) or iterate Object.keys(value) to avoid prototype-pollution issues and unexpected throws.
| if (value.hasOwnProperty(key)) { | |
| if (Object.prototype.hasOwnProperty.call(value, key)) { |
| /(?:github\.com\/)?([^\/\s]+)\/([^\/\s]+)\/pulls?\/(\d+)/, | ||
| ); | ||
| const ownerRepoMatch = text.match( | ||
| /(?:github\.com\/)?([^\/\s]+)\/([^\/\s]+)/, |
There was a problem hiding this comment.
Owner/repo extraction in GET_GITHUB_PULL_REQUEST still uses very broad ([^/\s]+) captures, which can include trailing punctuation/quotes (e.g., repo)), leading to invalid API calls. Other actions in this PR use the stricter [a-zA-Z0-9][\w.\-]* pattern to avoid this; consider aligning prMatch/ownerRepoMatch here to the same restricted pattern.
| /(?:github\.com\/)?([^\/\s]+)\/([^\/\s]+)\/pulls?\/(\d+)/, | |
| ); | |
| const ownerRepoMatch = text.match( | |
| /(?:github\.com\/)?([^\/\s]+)\/([^\/\s]+)/, | |
| /(?:github\.com\/)?([a-zA-Z0-9][\w.\-]*)\/([a-zA-Z0-9][\w.\-]*)\/pulls?\/(\d+)/, | |
| ); | |
| const ownerRepoMatch = text.match( | |
| /(?:github\.com\/)?([a-zA-Z0-9][\w.\-]*)\/([a-zA-Z0-9][\w.\-]*)/, |
Made-with: Cursor
Note
Medium Risk
Touches core
GitHubServiceand multiple actions/providers (validation, state merging, list/search output shaping), so regressions could affect command routing and returned data, though changes are mostly additive and guarded for mocks.Overview
Adds two dynamic providers,
MY_OPEN_PRSandMY_PR_STATUS, backed by a newGitHubService.getMyOpenPRs()method and an optionalGITHUB_PR_STATUS_ORG_FILTERsetting to scope results.Introduces a
GitHubServicedata enricher registry (registerEnricher,enrichItems) and wires list/search actions (PRs, issues, repos, branches) to pass results through enrichment when available; the plugin registers a built-in PR merge/review-status enricher used byMY_PR_STATUSand PR listing.Improves robustness around state and serialization: adds
state-helpersfor consistent repo/PR context lookup and return-state merging, addsutils/serializesanitizers to avoid circular refs in callback/state payloads, and deduplicates concurrentloadGitHubStatecache reads.Refines action validation/URL parsing (more intent-based validators, tighter owner/repo regexes, supports
/pulland/pulls) and updates docs/config (README, newCHANGELOG.md,.gitignore,package.jsonagent params), plus new unit tests for the enricher registry and PR status providers.Written by Cursor Bugbot for commit ab826fc. This will update automatically on new commits. Configure here.