feat: deprecate stuck statue and auto re-onboard stuck repos [CM-1098]#4034
feat: deprecate stuck statue and auto re-onboard stuck repos [CM-1098]#4034
Conversation
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
mbani01
changed the title
|
Your PR title doesn't contain a Jira issue key. Consider adding it for better traceability. Example:
Projects:
Please add a Jira issue key to your PR title. |
|
|
There was a problem hiding this comment.
Pull request overview
This PR removes the “stuck repository” concept from the git integration service by deleting the STUCK state/error and replacing stuck-handling with automatic re-onboarding, along with the associated DB/model cleanup.
Changes:
- Removes
RepositoryState.STUCK,ErrorCode.STUCK_REPO, and theStuckRepoErrorexception. - Updates the worker to treat “stuck” detection as a re-onboarding trigger (sets
PENDING_REONBOARD) rather than marking repos as stuck for manual intervention. - Drops the
stuckRequiresReOnboardcolumn fromgit."repositoryProcessing"and removes it from DB select/model mapping and acquisition exclusion logic.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| services/apps/git_integration/src/crowdgit/worker/repository_worker.py | Removes StuckRepoError handling and always queues stuck repos for re-onboarding. |
| services/apps/git_integration/src/crowdgit/models/repository.py | Removes stuck_requires_re_onboard field and DB mapping. |
| services/apps/git_integration/src/crowdgit/errors.py | Deletes StuckRepoError class. |
| services/apps/git_integration/src/crowdgit/enums.py | Removes STUCK repository state and STUCK_REPO error code. |
| services/apps/git_integration/src/crowdgit/database/crud.py | Removes stuckRequiresReOnboard from SELECT columns and stops excluding legacy STUCK state. |
| backend/src/database/migrations/V1776428661__dropStuckFlagFromRepositoryProcessing.sql | Drops the stuckRequiresReOnboard column from git."repositoryProcessing". |
| backend/src/database/migrations/U1776428661__dropStuckFlagFromRepositoryProcessing.sql | Present in PR metadata (no content shown in diff). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| states_to_exclude = ( | ||
| RepositoryState.PENDING, | ||
| RepositoryState.PROCESSING, | ||
| RepositoryState.STUCK, | ||
| RepositoryState.PENDING_REONBOARD, | ||
| RepositoryState.AUTH_REQUIRED, | ||
| ) |
There was a problem hiding this comment.
states_to_exclude no longer filters out the legacy stuck state. If there are still rows with state='stuck', they’ll now be picked up by the recurrent acquisition query. That can be fine, but it’s safer to explicitly migrate/normalize those rows to pending_reonboard (or another supported state) so the behavior is deterministic and doesn’t depend on historical data lingering in the table.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE git."repositoryProcessing" DROP COLUMN IF EXISTS "stuckRequiresReOnboard"; | |||
There was a problem hiding this comment.
This migration drops the stuckRequiresReOnboard column but doesn’t address existing rows that may still be in the removed state='stuck'. Since the app side removes the STUCK enum value, it’s worth adding a companion UPDATE in this migration (or a separate one) to rewrite any repositoryProcessing.state='stuck' rows to a supported state (e.g. pending_reonboard) to avoid leaving orphan/legacy states in production data.
| class RepositoryState(str, Enum): | ||
| """Repository processing states""" | ||
|
|
||
| PENDING = "pending" | ||
| PROCESSING = "processing" | ||
| COMPLETED = "completed" | ||
| FAILED = "failed" | ||
| REQUIRES_PARENT = "requires_parent" # fork repo without valid parent repo in out system | ||
| STUCK = "stuck" # requires manual resolution | ||
| PENDING_REONBOARD = "pending_reonboard" # re-onboarding deferred until weekend | ||
| AUTH_REQUIRED = "auth_required" # private repo or repo requiring authentication |
There was a problem hiding this comment.
With RepositoryState.STUCK removed, any existing git."repositoryProcessing".state = 'stuck' rows become an invalid enum value for code paths that deserialize rows into Repository (Pydantic will reject unknown enum values). Consider adding a DB/data migration to rewrite existing state='stuck' to an existing state (e.g. pending_reonboard), or add a backwards-compatible alias/mapping during deserialization so older rows don’t break runtime behavior.
![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 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 179c84d. Configure here.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE git."repositoryProcessing" DROP COLUMN IF EXISTS "stuckRequiresReOnboard"; | |||
There was a problem hiding this comment.
Migration doesn't convert existing stuck state rows
Medium Severity
The migration drops the stuckRequiresReOnboard column but doesn't update existing rows where state = 'stuck'. Since the STUCK enum value is removed from RepositoryState and STUCK is removed from states_to_exclude in acquire_recurrent_repo, these orphaned rows will be picked up as regular recurrent repos instead of being properly queued for re-onboarding (pending_reonboard). This means they go through a normal processing attempt (which may fail for the same reason they were stuck) and get marked FAILED, rather than being cleanly re-onboarded as the PR intends. The migration needs an UPDATE statement to convert existing state = 'stuck' rows to 'pending_reonboard'.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 179c84d. Configure here.
This pull request removes the concept of "stuck" repositories and related code from the git integration service. Instead of marking repositories as "stuck" and requiring manual intervention, repositories that are stuck in processing are now automatically queued for re-onboarding. This simplifies error handling and reduces the need for manual resolution.
Database and Model Cleanup:
stuckRequiresReOnboardcolumn from therepositoryProcessingtable and removed all references to this field from the codebase, including theRepositorymodel and database CRUD logic. [1] [2] [3] [4]Repository State and Error Handling:
STUCKstate fromRepositoryStateand theSTUCK_REPOerror code fromErrorCode, as well as theStuckRepoErrorexception class. [1] [2] [3] [4]repository_worker.pyto always raiseReOnboardingRequiredErrorand queue the repository for re-onboarding, instead of marking it as stuck.STUCKin the worker, since this state no longer exists.STUCKstate are no longer excluded.Note
Medium Risk
Changes repository processing state machine and DB schema, which can affect how repos are selected/handled and may alter operational behavior for long-running jobs. Risk is moderate due to migration and state transitions, but scope is contained to git-integration processing logic.
Overview
Deprecates the
STUCKrepository concept by removing theRepositoryState.STUCK, theSTUCK_REPOerror code, and theStuckRepoErrorpath.Stuck-processing detection in
repository_worker.pynow always triggersReOnboardingRequiredError, moving affected repos intoPENDING_REONBOARDinstead of leaving them in a manual-resolution state.Cleans up persistence and models by dropping
git.repositoryProcessing.stuckRequiresReOnboard(migration) and removing the field from repo SELECTs and theRepositorymodel DB mapping.Reviewed by Cursor Bugbot for commit 6459b74. Bugbot is set up for automated code reviews on this repo. Configure here.