Conversation
Handle `repository.pushed` in the cells, where repository commits are stored. Origin's payload includes the old and new ref tips but no commit list, so commits are read through the API. Only branch updates are recorded. Deletions and tags are skipped. A new branch records only its tip since its existing history is not part of the push.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a888b88. Configure here.
| return [] | ||
|
|
||
| if ref_update.created: | ||
| return [head_commit] if head_commit else [] |
There was a problem hiding this comment.
New branch tip dropped without headCommit
Medium Severity
A newly created branch records nothing when headCommit is missing, even though after is the tip SHA. Regular one-commit updates already fall back to the API in that case, so the create path leaves the tip unrecorded. Later pushes compare against that unrecorded tip and never pick it up.
Reviewed by Cursor Bugbot for commit a888b88. Configure here.
| installation = integration.get_installation(organization_id=repo.organization_id) | ||
| client = installation.get_client() |
There was a problem hiding this comment.
Bug: The _refresh_access_token function, called from a cell silo context, directly queries the control-silo-only Integration model, which will fail in environments without control-silo proxying configured.
Severity: HIGH
Suggested Fix
Ensure that any code path originating from a cell silo that needs to access the Integration model is explicitly proxied to the control silo. The @control_silo_function decorator does not provide this protection in production, so the logic must rely on the proxy client mechanism or another method to route the call correctly.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/integrations/cursor_origin/push.py#L88-L89
Potential issue: The `repository.pushed` webhook handler now runs in cell silos. Its API
client, `CursorOriginApiClient`, may attempt to refresh its access token by calling
`_refresh_access_token`. This function queries the `Integration` model, which only
exists in the control silo database. The `@control_silo_function` decorator on this
function is a no-op in production. If the environment is not configured to proxy
requests to the control silo, this code path will execute in the cell silo, causing a
database error when it tries to access the `Integration` model.
Did we get this right? 👍 / 👎 to inform future reviews.
| name = repo.config["name"] | ||
| comparison = client.compare_commits(name, ref_update.before, after) |
There was a problem hiding this comment.
Bug: The compare_commits API call may receive an empty string for the before parameter on existing branches, which could cause an unhandled API error if the external API rejects it.
Severity: MEDIUM
Suggested Fix
Before calling client.compare_commits, validate that ref_update.before is a non-empty string for branches where ref_update.created is false. If it is empty, log an error and return a 400 status, similar to how other payload validation errors are handled.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/integrations/cursor_origin/push.py#L115-L116
Potential issue: The `RefUpdate.from_payload` method defaults the `before` field to an
empty string if it's missing from the webhook payload. This potentially empty string is
then passed to `client.compare_commits` for non-created branches. If the external Origin
API rejects an empty string as a valid commit SHA, the API call will fail. Since there
is no specific error handling around this call, the exception will propagate up, causing
the webhook handler to return an HTTP 500 error.
Did we get this right? 👍 / 👎 to inform future reviews.
| elif status == "removed": | ||
| changes.append({"path": file["filename"], "type": "D"}) | ||
| elif status == "renamed": | ||
| changes.append({"path": file["previousFilename"], "type": "D"}) |
There was a problem hiding this comment.
Bug: A KeyError will occur if a renamed file from the Origin API is missing the optional previousFilename field, causing an unhandled exception in the webhook.
Severity: HIGH
Suggested Fix
Use the .get() method with a default value or check for the existence of the previousFilename key before accessing it. If the key is missing for a renamed file, it should be treated as a payload error and handled gracefully instead of causing a crash.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/integrations/cursor_origin/repository.py#L48
Potential issue: The `file_changes_from` function processes commit files from the Origin
API. For files with a status of `"renamed"`, it directly accesses the `previousFilename`
key using `file["previousFilename"]`. However, the type definition for this data,
`OriginCommitFile`, marks `previousFilename` as optional (`NotRequired`). If the Origin
API returns a renamed file without this key, a `KeyError` will be raised. This exception
is not caught by the specific `OriginPayloadError` handler and will instead be caught by
a generic `except Exception` block that re-raises, causing the webhook to crash.
Did we get this right? 👍 / 👎 to inform future reviews.


Handle
repository.pushedin the cells, where repository commits are stored. Origin'spayload includes the old and new ref tips but no commit list, so commits are read through
the API. Only branch updates are recorded. Deletions and tags are skipped. A new branch
records only its tip since its existing history is not part of the push.