thcipriani opened https://gitlab.wikimedia.org/repos/test-platform/catalyst/patchdemo/-/merge_requests/183

Treat patches marked DNM as open

I note that it does show up when you click "merged or abandoned" checkbox, but we count WIP patches as open when we set expiry dates.

I am inclined to agree that DNM is open.

Two options:

1. Call DNM open
2. Update checkbox label to "Wikis where all patches are merged, abandoned, or DNM"

I opted for option 1 in my change.

I think that wasn't the root cause, though -- the root cause was that a *dependency* (in this case, immediate parent) of the patch given to patchdemo was unmerged/open. So I was arguing that the patchdemo instance should not be considered 'all closed' while some of the *dependent* patches were still unmerged, even if all the explicitly listed patches had been merged/abandoned/DNM.

@cscott I was not able to reproduce with the existing patches. I also tried with a WIP patch since I see you mentioned it and still couldn't reproduce. I noticed that the example patchdemo has 9 revisions. Can you remember when you first saw the expiry date show up?

It's possible the expiry checker job changed the expiry date, but I did not see anything in the list of revisions that would have caused that and nothing in the logs of the last two expiry checker jobs (this could have happened in a previous job, but logs are unavailable now).

I do see two additional patch requests in the catalyst-api logs, without a subsequest request to stream logs, which means that the expiry date was likely changed at some point. I thought at first the time was close to the expiry checker's run time but after double checking, it doesn't seem to coincide with the expiry checker's run time. If one is in possession of an api key they could also make a request to catalyst to change the expiry date. This happened before the final revision.

[GIN] 2025/07/18 - 12:48:20 | 200 |   1.59035597s |       10.42.0.1 | PATCH    "/api/environments/1362"
[GIN] 2025/07/18 - 12:48:23 | 200 |  102.389774ms |       10.42.0.1 | GET      "/api/environments/1362?latestStatus=0"
[GIN] 2025/07/18 - 12:48:38 | 200 |   84.083174ms |       10.42.0.1 | GET      "/api/environments/1362?latestStatus=0"
[GIN] 2025/07/18 - 12:48:40 | 200 |  1.524394193s |       10.42.0.1 | PATCH    "/api/environments/1362"
[GIN] 2025/07/18 - 12:49:04 | 200 |   1.39868902s |       10.42.0.1 | GET      "/api/environments/1362?latestStatus=1"

I ran the expiry checker locally but no demos were affected. I've copied the example demo in production and will wait to see if the expiry date gets changed.

@jnuche I know that not updating the expiry date was intentional, but what do you think about this situation?

Right, since the patches can be changed when editing a wiki (or be set successfully for the first time if the wiki previously failed to install), the expiry date based on the patches/tasks state should also change. In my view the expiry date is also associated to the data created by the user in the wiki.

It should be possible to keep track of both things (patches/tasks state and data) but I feel like that would add significant complexity to our logic without much benefit. I'm ok with simply applying the expiry logic normally during an edit to avoid issues like this one; we have enough resources at the moment that I don't think it would be a problem to extend wiki lifetimes during edits.

Retitled based on the assumption that we found the root cause of the expiry date (e.g., create a wiki with no expiry date, edit it, editing fails, it gets the default expiry date).

@cscott if you remember the sequence of events and can confirm, that'd be cool :)

BUT(!) we found at least one way this can happen and it matches the logs from this case.

jhuneidi opened https://gitlab.wikimedia.org/repos/test-platform/catalyst/patchdemo/-/merge_requests/196

Update expiry date on rebuild

jhuneidi merged https://gitlab.wikimedia.org/repos/test-platform/catalyst/patchdemo/-/merge_requests/196

Update expiry date on rebuild