a8n: Fix deletion of repos that are associated with Campaigns/CampaignPlans

Created by: codecov[bot]

Codecov Report

Merging #7015 into master will decrease coverage by 0.02%. The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #7015      +/-   ##
=========================================
- Coverage   39.22%   39.2%   -0.03%     
=========================================
  Files        1230    1230              
  Lines       63534   63582      +48     
  Branches     6041    6041              
=========================================
+ Hits        24922   24926       +4     
- Misses      36328   36374      +46     
+ Partials     2284    2282       -2

Impacted Files	Coverage Δ
migrations/bindata.go	`0.45% <0%> (-0.03%)`
...d/frontend/internal/authz/bitbucketserver/store.go	`71.73% <0%> (+0.86%)`
internal/search/backend/horizontal.go	`90.32% <0%> (+1.61%)`

Created by: mrnugget

"The changesets remain on the code host, and this is indicated to them when deleting a campaign."

That's doable. I think we already have an alert box on deletion. cc @felixfbecker @eseliger

I would expect a comment to be added to the changeset on the code host with something like:

(Hmm, that's not in the comment linked above, or am I missing something?)

This would require adding the ability to comment on a PR. And then we'd have to add that to the deletion process, either in sync (which would make the delete slower, since we have to send N requests to GitHub/BBS) or asynchronously (which would then require polling/status check in case one of the N requests fails and we need to tell the user that the campaign has not been deleted)

Similarly, when a repository is removed by an admin, a comment should be added to the changeset:

That, too, is more work than it sounds like. Right now, repo-updater doesn't know anything about Automation. It "only" syncs repositories: get the desired list of repos from codehost, update the database. It does that pretty efficiently with a an UPSERT query.

If we want to inject behaviour in that ("before repo is deleted, close all associated changesets") we'd have to change the repo-updater syncer to call back to enterprise/a8n code and wait for that to close the changesets (which can slow down the syncer).

Or we "listen" to the events emitted by the syncer and then close the changesets of a repo. That would require, though, that we make the repo_id on changesets nullable and thus make the whole repository nullable in the GraphQL API, which is exactly this from the comment above:

As a future enhancement (not required immediately, we'll solicit customer feedback for priority), instead of removing the changesets from the campaign, they can be shown in a special "associated repository no longer exists" state in the campaign, so that the campaign author has confidence that the campaign is indeed affecting all of the repositories they originally intended to affect.

So, just to be clear, do you expect the three items (indication that changeset remains on codehost, add comment to changeset on codehost on campaign deletion, add comment to changeset on codehost on repo deletion) in your comment to be implemented?

(I was under the assumption that items 2 and 3 fall under "future enhancement (not required immediately, we'll solicit customer feedback for priority)" and we'd take a separate look at these after the current fix in this PR (which does fix the repo-updater that's currently broken) is shipped.

Created by: mrnugget

As an addition to me previous comment: the last point (add comment to changeset on codehost on repo deletion) and displaying changesets 'in a special "associated repository no longer exists' will become a lot easier once "repo soft deletion" is implemented, which @unknwon and @sourcegraph/core-services try to tackle after RFC 40: !6427 (closed)

Created by: unknwon

As an addition to me previous comment: the last point (add comment to changeset on codehost on repo deletion) and displaying changesets 'in a special "associated repository no longer exists' will become a lot easier once "repo soft deletion" is implemented, which @unknwon and @sourcegraph/core-services try to tackle after RFC 40: #6427 (comment)

FYI, repo soft delete issue is https://gitlab.sgdev.org/root/sourcegraph/-/issues/6859.

Created by: christinaforney

Ah, I interpreted "this is indicated to them" as the changeset will contain the context (via a comment) about where it came from.

So, just to be clear, do you expect the three items (indication that changeset remains on codehost, add comment to changeset on codehost on campaign deletion, add comment to changeset on codehost on repo deletion) in your comment to be implemented?

(I was under the assumption that items 2 and 3 fall under "future enhancement (not required immediately, we'll solicit customer feedback for priority)" and we'd take a separate look at these after the current fix in this PR (which does fix the repo-updater that's currently broken) is shipped.

All of these things can be done in follow-ups (obviously because this is merged already).

It sounds like we should tackle these individually (some of this is related to our roadmap discussion from this morning)

Most important for good user experience/meeting base level expectations (to prioritize in current work):

Add the ability to comment on a changeset in a code host
When closing a campaign, comment on the changeset to indicate that the campaign that created it has been closed.
Close the changeset on the code host when a campaign is deleted.

To be added to the backlog:

Add comment to changeset on code host on repo deletion from Sourcegraph
Notify admin when deleting a repo that is part of a campaign
Display changesets in special "associated repo no longer exists" way

Did I miss anything? Do you agree with the above priorities?

Created by: mrnugget

Most important for good user experience/meeting base level expectations (to prioritize in current work):

* Add the ability to comment on a changeset in a code host

* When closing a campaign, comment on the changeset to indicate that the campaign that created it has been closed.

* Close the changeset on the code host when a campaign is deleted.

Is there a reason for why we shouldn't implement "closing campaign" and "deleting campaign" exactly as described here in RFC 36? AFAICT these are the differences:

closing of changesets after closing campaign requires another explicit user action
no comments
deleting campaigns does not affect changesets on codehost

This is obviously easier to implement since we don't need to add comments and have explicit actions, which is why I prefer it. And I think we can already make progress on that in this iteration.

To be added to the backlog:

* Add comment to changeset on code host on repo deletion from Sourcegraph

* Notify admin when deleting a repo that is part of a campaign

* Display changesets in special "associated repo no longer exists" way

Just so you can take this into consideration when prioritizing/estimating this, I want to point out that the first two points here are a lot harder to implement than the last one.

That is due to our syncing process of repositories: there is never an imperative list of "repositories to add" or "repositories to delete", it's declaractive. A site-admin configures the external service config with repos and repositoryQuery and then we query the codehost to get a list of repositories that should be in the database. Every repository not in the list will get deleted.

Since the computation of the list happens asynchronously and can potentially take a really long time (it takes a while to fetch 100-item pages of repositories for 20k repositories): when would we notify the admin that a repository is to be deleted? Do we generate an error message? If so, when does that message appear and disappear?

Adding a comment is not a huge thing (since we could hook into the lifecycle of repos), but letting the admin know before a repo is deleted is, right now, really, really hard.

Created by: christinaforney

Follow up and continuation of thread from @mrnugget above is in #7082

a8n: Fix deletion of repos that are associated with Campaigns/CampaignPlans

Merged by (Oct 9, 2025 1:10am UTC)

Activity

Codecov Report

a8n: Fix deletion of repos that are associated with Campaigns/CampaignPlans

Merge request reports

Merged by (Oct 9, 2025 1:10am UTC)

Activity

Codecov Report