Add a new GraphQL mutation to sync with app token

[michelletran-sentry]

This change makes the sync task use an app token rather than the user's token. It's a separate mutation, so it shouldn't conflict with the existing sync trigger.

Just also note that the current_owner is set to be the org with the JWT middleware:

umbrella/apps/codecov-api/codecov_auth/middleware.py

Line 200 in 3541fda

request.current_owner = owner

.

[seer-by-sentry]

Sentry detected 3 potential issues in your recent changes

There's a suspicion that the new syncRepos mutation might encounter an AttributeError if current_owner is None due to bypassed validation, potentially causing unexpected behavior.

• Description: The resolve_sync_repos function, when using_integration=True, bypasses validation that would normally ensure current_owner is not None. The RefreshService().trigger_refresh method then directly accesses self.current_owner.ownerid and self.current_owner.username. If current_owner is None (e.g., due to authentication failure or anonymous user), this will result in an AttributeError: 'NoneType' object has no attribute 'ownerid', causing the GraphQL request to crash. For example, if info.context["executor"].current_owner is None, the subsequent access self.current_owner.ownerid will fail. Unlike existing mutations, this new one lacks a general error handling decorator. Example: RefreshService().trigger_refresh(None.ownerid, None.username, ...).
• Code location: apps/codecov-api/graphql_api/types/mutation/sync_repos/sync_repos.py:1~4
• Suggested fix: Add a null check for current_owner before accessing its attributes in RefreshService.trigger_refresh or ensure proper authentication validation is not bypassed. Consider adding @wrap_error_handling_mutation.

It's suspected that the SyncReposTask, when triggered by the new syncRepos mutation, lacks error handling for NoConfiguredAppsAvailable or InvalidInstallationError from GitHub App token retrieval, potentially leading to Celery task crashes.

• Description: The SyncReposTask, specifically when using_integration=True (triggered by the new GraphQL mutation), calls get_owner_provider_service which can lead to NoConfiguredAppsAvailable or InvalidInstallationError exceptions during GitHub App token retrieval. For instance, if all configured GitHub Apps are rate-limited or suspended, NoConfiguredAppsAvailable is raised. The SyncReposTask currently has a try/except for LockError but no handling for these specific GitHub App related exceptions. This means such exceptions will bubble up, causing the Celery task to crash unexpectedly. Historical Sentry data confirms these exceptions occur in production. Example: git = get_owner_provider_service(...) raises NoConfiguredAppsAvailable, which is unhandled.
• Code location: apps/worker/tasks/sync_repos.py:88~92
• Suggested fix: Implement robust try/except blocks in SyncReposTask to catch NoConfiguredAppsAvailable and InvalidInstallationError exceptions, logging them appropriately and handling task failure gracefully.

It is suspected that if current_owner.user is None, accessing self.current_user.is_authenticated in BaseInteractor will cause an AttributeError. This risk is heightened by the new mutation.

• Description: The exact failure mechanism is an AttributeError: 'NoneType' object has no attribute 'is_authenticated'. This occurs when self.current_user becomes None in the BaseInteractor constructor (codecov/commands/base.py:28-29) because self.current_owner.user is None. The Owner model allows user to be null. For example, if current_owner.user is None, then self.current_user will be None. Subsequently, any call to self.current_user.is_authenticated (e.g., in TriggerSyncInteractor.validate()) will raise the AttributeError. The new mutation increases the likelihood of this by potentially setting current_owner via JWT middleware to an organization without a linked user. Example: if None.is_authenticated: ...
• Code location: apps/codecov-api/codecov_auth/commands/owner/interactors/trigger_sync.py:15~16
• Suggested fix: Ensure self.current_user is always a valid User object or AnonymousUser in BaseInteractor's constructor, even when self.current_owner.user is None. Implement a check for self.current_user is None before accessing its attributes.

_{Did you find this useful? React with a 👍 or 👎}

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.31%. Comparing base (4256b1f) to head (f69ae13).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #202   +/-   ##
=======================================
  Coverage   94.31%   94.31%           
=======================================
  Files        1230     1232    +2     
  Lines       45306    45325   +19     
  Branches     1449     1449           
=======================================
+ Hits        42732    42750   +18     
- Misses       2270     2271    +1     
  Partials      304      304           

Flag	Coverage Δ
apiunit	96.49% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[codecov-notifications]

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

[ajay-sentry]

Thoughts on a comment here to let folks know these are both triggering the sync task, one is just using integration and the other isn't?

[michelletran-sentry]

Added a comment to sync_repos.py

[ajay-sentry]

nit: this should be required right?

[ajay-sentry]

with an "!" I mean

[michelletran-sentry]

Oh, it doesn't work with errors. I'll need to have that be nullable in case there is an error.

[ajay-sentry]

ohhh gotcha

[ajay-sentry]

so we either return isSyncing or the error in the payload

[ajay-sentry]

lgtm, one small comment