Fix prebuild lookup for prebuilds run in reverse-chronological order

[filiptronicek]

Description

The following situation breaks Gitpod as it is today:

1. A user commits two commits: A and B to main
2. A user opens a workspace for B (the current head of main) and that triggers a prebuild
3. A user opens a workspace for A (either by it being the head of another branch, or it being a directly opened commit). This in turn triggers a prebuild for A.
4. A user tries to open main (commit B) again as a new workspace, but now the prebuild for A would get used, because it was created later than the one for B and therefore returned first from the DB.

How to test

See updates to DB tests.

/hold

[filiptronicek]

Before merging, I will investigate if our assumption of "commits are ordered in descending chronological order" holds. This will entail going through API docs of our SCM providers

[geropl]

I had a closer look, and sadly something seems to be off about the test: If I undo the changes, it still works 😢

Update: I re-tested, and everything worked as expected. Not sure what went wrong yesterday; might have forgotten the yarn watch and testing outdated code 🤷

[filiptronicek]

@geropl I've pushed f4eac6d (#20360), which does a bunch of things to simplify and more thoroughly explain the matching process. I would love to hear what you had in mind with the restructure, since after giving it some thought, I think this pattern of having a method returning a match is not too bad.

[geropl]

@filiptronicek Sorry for the confusion: I did not mean to argue against having a filter function, that certainly makes sense.

Also, I realized when looking at your solution, that my idea from yesterday did not make sense. 🙄

Synthesizing our approaches, I took the liberty to push 29e6aa6. Happy to sync on this! 👋

[filiptronicek]

@geropl that filter function was a bit redundant anyway, since we check for commit overlap in isMatchForIncrementalBuild already.

I think these two should be identical, is there anything in particular that made you add the warning? Either way, we should definitely unify which one we use in the code.

gitpod/components/server/src/prebuilds/incremental-workspace-service.ts

Lines 173 to 176 in 29e6aa6

	// TODO(gpl) Isn't "candidateCtx.revision" identical to "candidatePrebuild.commit"? If yes, we could do .indexOf once...
	if (candidateCtx.revision !== candidatePrebuild.commit) {
	log.warn("Prebuild matching: commits mismatch!", { candidateCtx, candidatePrebuild });
	}

[geropl]

I think these two should be identical

I think so, too, but am not 💯 sure I'm not missing sth. So going with the log, and a follow-up to remove it if it does not show up the next week. 👍

[geropl]

Code LGTM, tests are green (+ looking great!): ✔️