fix(sound): guard recording lifecycle

[martin-henz]

Re-opens #839, which was automatically closed (not merged) when its base branch conductor-migration was deleted after being merged into master via #680. This branch cherry-picks the original commits from @11suixing11 unchanged, onto current master.

Summary

• reserve one in-flight recording slot synchronously when record or record_for accepts a request
• make record's returned stop function idempotent so repeated calls reuse one stopRecording() promise
• release recording state after successful completion and after startRecording/stopRecording failures
• guard cleanup with a per-recording generation token so a stale stop() cannot release a newer reservation

Fixes #799

Why

record and record_for schedule delayed work before io().startRecording() runs. The existing activePlayCount check prevents recording during playback, but there was no state reserved for a recording request itself. Two quick calls could therefore schedule independent timeout chains and eventually attempt overlapping recordings against the same tab-side recorder.

The PR reserves recording state at call time, so a second request fails before either timeout chain reaches startRecording().

Review Follow-up

CodeRabbit flagged a stale cleanup edge case: after startRecording() rejects and releases the reservation, invoking the old returned stop() could settle a second cleanup after a newer recording had reserved the mic.

reserveRecording() now returns a generation token, and releaseRecording() only clears state when that token still identifies the active reservation. The regression test covers this exact sequence:

1. the first record() fails during startup
2. a newer recording reserves the mic
3. the stale stop() from the failed first recording is invoked
4. the newer reservation remains active

Testing (from original PR, re-verify on CI)

• yarn workspace @sourceacademy/bundle-sound test (71 passed)
• yarn workspace @sourceacademy/bundle-sound tsc
• yarn workspace @sourceacademy/bundle-sound lint

Original author: @11suixing11

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[martin-henz]

Reviewed the diff and checked out the branch to verify directly. This is a careful, well-tested concurrency fix — no issues found.

The bug (#799): record/record_for schedule their pre-recording delay/signal chain and only call io().startRecording() after it resolves. Before this PR, the only guard was activePlayCount > 0 (unrelated to recording) plus a mic-permission check — there was no "a recording request is already in flight" state at all. So two record()/record_for() calls made back to back (before either one's delay chain reached startRecording()) could each independently schedule their own timeout chain and both eventually call io().startRecording() against the same tab-side recorder.

The fix: reserveRecording()/releaseRecording() centralize the guard, and reservation happens synchronously at call time (globalVars.recordingInProgress = true before any await), so a second call in the same tick sees the flag already set and throws immediately — confirmed by expect(io.startRecording).not.toHaveBeenCalled() in the new "reserves recording state synchronously" tests for both record and record_for.

Tracing the release paths for record(), since this is the trickiest part (a lazily-constructed, idempotent stop() plus a separately-catchable started chain):

• started.catch(() => { if (!recordingDone) releaseRecording(generation); }) is attached synchronously right after started is created, so there's no window where a rejection could go unhandled before the guard exists.
• If started rejects and stop() was never called, that catch handles the release (since recordingDone is still undefined).
• If stop() was already called, recordingDone is set, so that catch's release is a no-op, and release instead happens via recordingDone's own .finally(() => releaseRecording(generation)) once the (now-rejected) chain settles.
• If stop() is called after started already rejected (the stale-stop case CodeRabbit flagged on the original PR), recordingDone gets constructed from the already-rejected started, propagates the rejection, and its .finally fires releaseRecording(generation) again — a second call with a stale generation. Because releaseRecording only clears state if (recordingGeneration === generation), this second call is a correct no-op once a newer record()/record_for() has bumped the counter. This is exactly what the "a stale stop after a failed start cannot release a newer recording" test exercises, and it's the right fix for that edge case — not just papering over the symptom.
• I checked for a race between the synchronous recordingDone = ... assignment inside stop() and the started.catch microtask: since the assignment is synchronous (no await before it), there's no way for the catch microtask to interleave mid-assignment, so !recordingDone is always read consistently.

record_for is simpler (single try/finally wrapping the whole sequence, no external stop() to make idempotent), and releases exactly once when the async IIFE settles either way — no edge cases there.

Provenance check: the PR body's account of "CodeRabbit flagged a stale cleanup edge case" checks out — I looked at the original #839's comment history and CodeRabbit's walkthrough is there, followed by the author's own commit message describing the exact same fix now present here (reserveRecording() returning a generation token). This is a genuine second round of review-driven refinement, not a fabricated changelog.

Verified locally (checked out replace-pr-839):

• yarn workspace @sourceacademy/bundle-sound test — 71/71 passing, matches the PR's claim
• yarn workspace @sourceacademy/bundle-sound tsc — clean
• yarn workspace @sourceacademy/bundle-sound lint — same 6 pre-existing warnings seen on master (environment-dependent rule, off outside CI), nothing new

No changes requested from me — this is a solid fix for a real race, and the test suite specifically targets the subtle generation-token edge case rather than just the happy path.

[martin-henz]

Thanks @11suixing11 for this contribution. Good work!