fix: Make sure an alarm can't call onStop in the middle of a start() call

[gabivlj]

We are also making sure onStop can't set 'stopped' if the method calls start().

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@cloudflare/containers@218

commit: a5fd50a

[ask-bonk]

I'm Bonk, and I've done a quick review of your PR.

The fix protects in-flight start() from racing with alarm()-triggered onStop by (a) deferring state.setRunning() until after container.start() and (b) only writing stopped in callOnStop if the cached state object reference hasn't been replaced. Logic looks correct. A few items to address:

1. Missing changeset (high) - this is a user-facing behavior fix in Container.start() / callOnStop.
2. Reference-equality invariant in setStoppedIfUnchanged is load-bearing and undocumented (medium) - silently breaks if ContainerState ever mutates this.status in place instead of reassigning.
3. start() no longer sets state to running when the container is already running (low/medium) - subtle behavior change vs. previous code; worth confirming intentional.

[ask-bonk]

Review posted successfully on PR #218. Summary of feedback:

1. Missing changeset - User-facing change in Container.start()/onStop semantics needs a .changeset/*.md per AGENTS.md.
2. Undocumented reference-equality invariant in setStoppedIfUnchanged - relies on every state setter reassigning this.status; will silently break if anyone later refactors to in-place mutation. Suggested adding a doc comment.
3. Subtle behavior change: start() no longer overwrites state to 'running' when the container is already running.
4. Note on onStop throw path in callOnStop (informational, not a regression).
5. Test gap: the alarm test does not actually exercise setStoppedIfUnchanged because state starts as 'stopped'; suggested mocking storage.get to 'running' so a future regression that drops the guard fails.

github run

[ask-bonk]

LGTM

github run

[ask-bonk]

I've already posted a thorough review on this PR (comments from ask-bonk covering changeset, reference-equality invariant, behavior change, throw path, and test gap). The author has responded and addressed/acknowledged my comments. There's nothing new to review.

LGTM

github run