Nice use of helper functions, kept the tests focused on the relevant parts! 😎

Because builds are idempotent, we shouldn't need to switch off the last transition. Weshould be able to just start or stop.

Even so, it's probably no harm for this thing to make a best-effort at being idempotent as well.

Ahh yes, but I mean going off the last transition could lead to unexpected results if I'm reading correctly.

eg. I'm in the stopped state but the workspace was updated, now my autostop wouldn't trigger to update my workspace. I'm not sure if this is a problem, but the behavior could be odd to customers.

So I wrote a test to see what happens right now in this state:

• Given: we have a stopped workspace with autostart enabled
• Also given: the TemplateVersion of the Template used by the workspace changes

In this case, we do trigger an autostart, but with the last template version used by the workspace.

As this is an automated process, I think this makes sense and we would want the user to manually update the template version if the workspace is outdated. In future, we may want to allow users to opt-in to automatically updating workspaces to the latest version.

I wouldn't be opposed to exposing a function like:database.CreateWorkspaceBuild. I'm not a massive fan either, because it certainly conflicts withdatabase.InsertWorkspaceBuild. 🤷

I'm pausing extracting a function here because we only have two datapoints right now; I'd like to see at least one more before doing so.

I like that!

suggestion(if-minor): Just one suggestion: ticket > todo. Entirely up to you, won't hold up on that.

@vapurrmaid good call:#1401

We haven't the_ idiom in other places in the codebase. I'm impartial as to whether it's good or not, but we could remove them for consistency in this case!

What do you think about calling the lifecycle packageautobuild orcronbuild? I'm concerned about calling itlifecycle, since that term could be interpreted very broadly.

autobuild is better and does what it says on the tin.

comment(in-support):autobuild seems intuitive/useful to me.

Since thisRun function is always called in a goroutine, could we do that automatically for the caller inNew()?

smallest nit possible: We could drop thedo here

Comment(general): Your code looks great. I'm feeling a bit lost in understandingall that's happening in this"Start a Coder server"RunE because it's growing quite large with some branching andGoroutines. I'm wondering if that is just a me thing, or if maybe the complexity here is growing large. To understand what I mean, this diff in isolation looks harmless. In the grand scheme of initializing everything within thisRunE, though, it's harder to really evaluate.

I'm under the impression we could extract out some initialization steps to named functions and call them in a pipeline, but maybe that's just a me thing and not the general preferred approach for Go + Cobra. Do you have any comments to that effect?

Regardless, this comment is not meant for your code/this PR, just a general observation.

Agreed, I'd be in favour of extracting some initialization logic to make things slightly more concise, just would want to be careful to avoid too many layers of indirection.

comment(in-support):autobuild seems intuitive/useful to me.

suggestion(if-minor): Just one suggestion: ticket > todo. Entirely up to you, won't hold up on that.

Praise: Thank you for thinking of these files. We should ensure that we add matching comments to each 3 of these files to consider adding entries in all 3. It's easy to miss/forget.